Allow Creating dataset with default license via API regardless of allow custom terms setting by qqmyers · Pull Request #12551 · IQSS/dataverse

qqmyers · 2026-07-21T21:28:48Z

What this PR does / why we need it: Dataverse currently creates an incomplete/invalid dataset with no license and no terms of use when a dataset is created by API, the user doesn't specify a license or terms, and the :AllowCustomTerms setting is true.

This PR adds a new Feature Flag dataverse.feature.do-not-assume-default-license that allows forcing such a dataset to not get the default license, regardless of whether custom terms are allowed, with a new default that the default license is assigned regardless of whether custom terms are allowed or not.

This serves two use cases:

being able to assure that a dataset gets the default license even if custom terms are allowed,
not assigning a license that the user may not be aware of when the dataset is created via API, forcing the user to select a license/terms before the dataset can be published.

Which issue(s) this PR closes:

Closes Via API, datasets can be created in an invalid state when it comes to license/terms #11161

Special notes for your reviewer: This PR also tightens/object-orients the constraint that a dataset can't have a license and terms of use (or other similar fields). Before that was constrained only by the callers when setting terms of use but was handled in the TofUandA class itself when a license was set. Now both directions are constrained in the class.

Suggestions on how to test this: The basic logic is tested in the new JSON parser test. A Harvard use case for this is to use the default and assure that the HEAL bridge creates datasets with the default license when Dataverse is configured to allow custom terms. See #368. Setting the FeatureFlag true should restore the current default behavior and also leave the license unassigned when custom terms are not allowed. It would also be good to test that when custom terms aren't allowed, the UIs don't break when a dataset has no license (nominally we show a warning that a license or terms are needed when custom terms are allowed, it's possible that we don't handle this case).

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?: included

Additional documentation:

Preview docs:

…nment

pdurbin

I say we make the improved behavior opt-out instead.

pdurbin · 2026-07-22T12:22:17Z

+- When set to `true`, the default license will be automatically assigned if no license and no terms are provided in the input JSON.
+- When set to `false`, no license (and no terms) will be assigned if none are provided.
+- When not set (the default), the system maintains backward compatibility: the default license is assigned if custom terms are not allowed; if custom terms are allowed, no license is assigned.


I think the default should be the new behavior: give the dataset the default license.

And sure, have a setting for opt-ing out of the new, better behavior, if we feel like it's valuable.

Not sure what that would be called if it stays binary true/false. The two states when the new setting is set would then be:

don't assume default license if custom terms are allowed

never assume the default license

with always assume the default license being the new default. Any suggestions?

You could reverse the logic and call it dataverse.api.allow-no-license

The new term would have to describe the two states that aren't the new default (top/bottom lines).

~nevermind - the problem is only if we need to handle three states, but since any given instance either does/doesn't allow custom terms, there's no one who needs the current capability. I'll reactor...

…cense # Conflicts: # doc/sphinx-guides/source/installation/config.rst

pdurbin

I'm happy with the direction. I like the new default of not letting a dataset be created without a license (not that I've tested it). I did find a few items to address that I'm putting in this review. I'm happy to do a call about them.

Update, I did some light testing and the feature flag seems to work well. I pushed a commit to my fork: pdurbin@03cd5fb

pdurbin · 2026-07-29T20:05:42Z

+        TermsOfUseAndAccess copy2 = terms.copyTermsOfUseAndAccess();
+        assertNull(copy2.getLicense());
+        assertEquals("Some terms", copy2.getTermsOfUse());
+    }


The following two tests are failing, according to https://github.com/IQSS/dataverse/actions/runs/29957008615/job/89048490424?pr=12551

[ERROR] DatasetVersionDifferenceTest.testDifferencing:192->compareResults:274 expected: <1> but was: <2>

[ERROR] FileUtilTest$FileUtilNoParamTest.testIsDownloadPopupRequiredHasTermsOfUseAndCc0License:113 expected: <false> but was: <true>

Please fix.

One test was creating an illegal version (with termsOfUse and a license), so I adjusted the test to account for the license or custom terms restriction (~ the reason for putting the restriction in the class itself, so we can't have code/tests creating illegal objects). The other test was to explicitly test whether a popup appears when you have both a license and termsOfUse. Since that is no longer allowed, I deleted that test.

pdurbin · 2026-07-29T20:08:04Z

+        if (termsOfUse != null) {
+            this.license = null;
+        }


I find it a little odd, a bit of a code smell perhaps, that this logic is in the setter. (Not just this setter, but all the setters below too.)

And if we add a new item (setFoobarRestrictions), we have to remember to add the logic there too?

Should we at least add some javadoc somewhere to explain the need for this logic in all the setters?

AI calls this "Encapsulated Setter Logic" and says it's a reasonable approach. We were already using it in setLicense (where we clearCustomTermsVariables()), this PR adds the restriction in the other direction. I've gone ahead and added a note on setLicense and the setters for the terms that affect the license.

Thanks. I asked a follow up question here: https://github.com/IQSS/dataverse/pull/12551/changes#r3683535892

Another reasonable approach is to have POJOs. 😄 If we move to records some day, there are no setters!

Co-authored-by: Philip Durbin <philip_durbin@harvard.edu>

Changes were made.

pdurbin · 2026-07-30T14:23:23Z

+        assertNull(copy2.getLicense());
+        assertEquals("Some terms", copy2.getTermsOfUse());
+    }
+}


I'm seeing this error:

[ERROR] Not authorized or project not found. Please check the 'SONAR_TOKEN' environment variable, the 'sonar.projectKey' and 'sonar.organization' properties, or contact the project administrator to verify the token's permissions. #12570

@qqmyers should we have you set up SONAR_TOKEN and friends in your fork?

@poikilotherm would be the person to ask - doesn't seem scalable to ask that of all forks, but it could probably be done for this one or for ones for core team members, etc.

Ok, I added it to the agenda for tech hours: https://docs.google.com/document/d/1xVD_Xi5OYlJnU1scEDQv5jjEIovu1GZDuilxwpgorpo/edit?usp=sharing

I also pinged him here: https://iqss.slack.com/archives/C010LA04BCG/p1785427834898959?thread_ts=1784727464.778779&cid=C010LA04BCG

The underlying issue is well known: PRs from forks have no access to secrets.
I'm not sure why the coveralls coverage submission job is failing silently, but oh well...

From here, we have two options:
A) Only run the Sonar job on branches and PRs originating from IQSS
B) Run the Sonar job in a special environment, so we can have access to secrets and have Sonarcloud analysis on community (fork) PRs.

WDYT?

@poikilotherm can you please say more about option B? Sound great! Is it simple to set up? Is it free? 😅

Yeah, it's all free, but it's not simple. The hardest part: while we run any of this, we need to collect the results from the different test that ran. This is easy to do if within the flow, but harder when we have to figure out which runs may have something to report.

Aside from that, there is an option to use a comment-command (see also how we do preview images from forks), we could use the pull_request_target trigger (kind of endangering secrets when running unmoderated) or some other clever trigger around "workflow_run" (example).

If you query a search engine for "Sonarcloud", "fork" and "PR", you will find quite a bunch of people stuck with the same problem and not a lot of solutions. Unfortunately, neither of them being "easy" or "simple".

BTW - The official way to solve this is "Automatic Analysis", where scanning happens on the Sonarcloud side of things. Unfortunately, our codebase is way past their limits - we must use the CI method.

Ok, should we just go for option A for now? So contributors aren't confused by the failing check?

Thanks, @poikilotherm!

@qqmyers I just merged this PR from Oliver:

Limit SonarQube job to upstream repository #12576

Do you mind if I rebase PR or merge develop? Or you're welcome to. That should make the failing Sonar check go away.

Jim merged the latest and the Sonar check is no longer failing because it is now skipped for forks.

pdurbin · 2026-07-30T14:28:48Z

+            terms.setTermsOfUse(obj.getString("termsOfUse", null));
+            terms.setConfidentialityDeclaration(obj.getString("confidentialityDeclaration", null));
+            terms.setSpecialPermissions(obj.getString("specialPermissions", null));
+            terms.setRestrictions(obj.getString("restrictions", null));
+            terms.setCitationRequirements(obj.getString("citationRequirements", null));
+            terms.setDepositorRequirements(obj.getString("depositorRequirements", null));
+            terms.setConditions(obj.getString("conditions", null));
+            terms.setDisclaimer(obj.getString("disclaimer", null));


Am I right in guessing that it's important to set the license (or at least attempt to) BEFORE calling these lines?

That's my read of what's going on in the setters anyway: https://github.com/IQSS/dataverse/pull/12551/changes#r3677713314

If so, if the order is important, when to call setLicense and when to call setTerms and friends, should we have a comment about this?

I don't think so - both the license and terms fields all do a null check so they don't affect the other field(s) unless you send in a non-null value. As far as I can tell, in both this and the original code, if you send both a license and terms, the license wins silently - might be good to fail with a 400 instead, but perhaps not related to this PR.

Ok, thanks for clarifying!

pdurbin · 2026-07-30T14:32:51Z

+        datasetVersion2.getTermsOfUseAndAccess().setTermsOfAccess("Terms o' Access");
        String[] termField = new String[] {
-                BundleUtil.getStringFromBundle("file.dataFilesTab.terms.list.termsOfUse.header"), "", "Terms o' Use" };
+                BundleUtil.getStringFromBundle("file.dataFilesTab.terms.list.termsOfAccess.termsOfsAccess"), "", "Terms o' Access" };


Is "termsOfsAccess" a typo? It seems like it. 🤔 Pre-existing, I guess.

I assume. It's only used about 15 places :-)

D'oh! Ok. We'll fix it later.

pdurbin

I'm approving this. I only did some light testing with some IT tests I wrote on the side.

It's a known issue that the new SonarQube check is failing. We're discussing it.

…cense

qqmyers added 2 commits July 21, 2026 17:07

new setting to determine create behavior w.r.t. default license assig…

0f724e9

…nment

release note

e8716e5

qqmyers added this to IQSS Dataverse Project Jul 21, 2026

qqmyers moved this to Ready for Triage in IQSS Dataverse Project Jul 21, 2026

qqmyers moved this from Ready for Triage to Ready for Review ⏩ in IQSS Dataverse Project Jul 21, 2026

qqmyers added the Size: 10 A percentage of a sprint. 7 hours. label Jul 21, 2026

pdurbin self-assigned this Jul 22, 2026

pdurbin moved this from Ready for Review ⏩ to In Review 🔎 in IQSS Dataverse Project Jul 22, 2026

pdurbin requested changes Jul 22, 2026

View reviewed changes

pdurbin assigned qqmyers Jul 22, 2026

only support full on/off

2b69f80

qqmyers removed their assignment Jul 22, 2026

qqmyers added 3 commits July 22, 2026 12:43

Merge remote-tracking branch 'IQSS/develop' into IQSS/11161-defaultLi…

9a34fe1

…cense # Conflicts: # doc/sphinx-guides/source/installation/config.rst

fix tests, fully enforce no license and terms in TofU&A

a018fdb

move DatasetType logic before setLicense()

f9b46f3

qqmyers added Feature: Metadata Original size: 10 Type: Bug a defect UX & UI: Design This issue needs input on the design of the UI and from the product owner UX/UI Input Needed Apply to issues involving UX or UI implications that need additional input labels Jul 23, 2026

cmbz added the FY27 Sprint 2 FY27 Sprint 2 (2026-07-15 - 2026-07-29) label Jul 29, 2026

qqmyers added this to the 6.12 milestone Jul 29, 2026

pdurbin previously requested changes Jul 29, 2026

View reviewed changes

cmbz added the FY27 Sprint 3 FY27 Sprint 3 (2026-07-29 - 2026-08-12) label Jul 29, 2026

pdurbin assigned qqmyers Jul 29, 2026

qqmyers and others added 4 commits July 29, 2026 17:36

fix test - was using illegal TofUandA

5cfcb49

remove obsolete test - can't have license and TofU

e7f0751

note the enforcement of license OR custom terms on setters

c361b33

Apply suggestion from @pdurbin

8975844

Co-authored-by: Philip Durbin <philip_durbin@harvard.edu>

qqmyers removed their assignment Jul 29, 2026

pdurbin mentioned this pull request Jul 30, 2026

[ERROR] Not authorized or project not found. Please check the 'SONAR_TOKEN' environment variable, the 'sonar.projectKey' and 'sonar.organization' properties, or contact the project administrator to verify the token's permissions. #12570

Closed

pdurbin reviewed Jul 30, 2026

View reviewed changes

pdurbin assigned qqmyers Jul 30, 2026

pdurbin reviewed Jul 30, 2026

View reviewed changes

qqmyers removed their assignment Jul 30, 2026

pdurbin approved these changes Jul 30, 2026

View reviewed changes

github-project-automation Bot moved this from In Review 🔎 to Ready for QA ⏩ in IQSS Dataverse Project Jul 30, 2026

pdurbin removed their assignment Jul 30, 2026

Merge remote-tracking branch 'IQSS/develop' into IQSS/11161-defaultLi…

711b0b8

…cense

Uh oh!

Conversation

qqmyers commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pdurbin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pdurbin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pdurbin Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

poikilotherm Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pdurbin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

qqmyers commented Jul 21, 2026 •

edited

Loading

pdurbin left a comment •

edited

Loading

pdurbin Jul 30, 2026 •

edited

Loading

poikilotherm Jul 30, 2026 •

edited

Loading