Skip to content

Commit

Permalink
Merge pull request #8697 from ErykKul/8671_license_sorting
Browse files Browse the repository at this point in the history
8671 license sorting
  • Loading branch information
kcondon authored Nov 21, 2022
2 parents 11e7505 + 3a9518f commit 948c0d0
Show file tree
Hide file tree
Showing 24 changed files with 157 additions and 24 deletions.
7 changes: 7 additions & 0 deletions doc/release-notes/8671-sorting-licenses.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## License sorting

Licenses as shown in the dropdown in UI can be now sorted by the superusers. See [Configuring Licenses](https://guides.dataverse.org/en/5.10/installation/config.html#configuring-licenses) section of the Installation Guide for reference.

## Backward Incompatibilities

License files are now required to contain the new "sortOrder" column. When attempting to create a new license without this field, an error would be returned. See [Configuring Licenses](https://guides.dataverse.org/en/5.10/installation/config.html#configuring-licenses) section of the Installation Guide for reference.
3 changes: 2 additions & 1 deletion doc/sphinx-guides/source/_static/api/add-license.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"uri": "http://creativecommons.org/licenses/by/4.0",
"shortDescription": "Creative Commons Attribution 4.0 International License.",
"iconUrl": "https://i.creativecommons.org/l/by/4.0/88x31.png",
"active": true
"active": true,
"sortOrder": 2
}
9 changes: 8 additions & 1 deletion doc/sphinx-guides/source/api/native-api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4015,7 +4015,7 @@ View the details of the standard license with the database ID specified in ``$ID
curl $SERVER_URL/api/licenses/$ID
Superusers can add a new license by posting a JSON file adapted from this example :download:`add-license.json <../_static/api/add-license.json>`. The ``name`` and ``uri`` of the new license must be unique. If you are interested in adding a Creative Commons license, you are encouarged to use the JSON files under :ref:`adding-creative-commons-licenses`:
Superusers can add a new license by posting a JSON file adapted from this example :download:`add-license.json <../_static/api/add-license.json>`. The ``name`` and ``uri`` of the new license must be unique. Sort order field is mandatory. If you are interested in adding a Creative Commons license, you are encouarged to use the JSON files under :ref:`adding-creative-commons-licenses`:

.. code-block:: bash
Expand All @@ -4040,6 +4040,13 @@ Superusers can delete a license, provided it is not in use, by the license ``$ID
.. code-block:: bash
curl -X DELETE -H X-Dataverse-key:$API_TOKEN $SERVER_URL/api/licenses/$ID
Superusers can change the sorting order of a license specified by the license ``$ID``:

.. code-block:: bash
export SORT_ORDER=100
curl -X PUT -H 'Content-Type: application/json' -H X-Dataverse-key:$API_TOKEN $SERVER_URL/api/licenses/$ID/:sortOrder/$SORT_ORDER
List Dataset Templates
~~~~~~~~~~~~~~~~~~~~~~
Expand Down
23 changes: 23 additions & 0 deletions doc/sphinx-guides/source/installation/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1092,6 +1092,29 @@ Disabling Custom Dataset Terms

See :ref:`:AllowCustomTermsOfUse` for how to disable the "Custom Dataset Terms" option.

.. _ChangeLicenseSortOrder:

Sorting licenses
----------------

The default order of licenses in the dropdown in the user interface is as follows:

* The default license is shown first
* Followed by the remaining installed licenses in the order of installation
* The custom license is at the end

Only the order of the installed licenses can be changed with the API calls. The default license always remains first and the custom license last.

The order of licenses can be changed by setting the ``sortOrder`` property of a license. For the purpose of making sorting easier and to allow grouping of the licenses, ``sortOrder`` property does not have to be unique. Licenses with the same ``sortOrder`` are sorted by their ID, i.e., first by the sortOrder, then by the ID. Nevertheless, you can set a unique ``sortOrder`` for every license in order to sort them fully manually.

The ``sortOrder`` is an whole number and is used to sort licenses in ascending fashion.

Changing the sorting order of a license specified by the license ``$ID`` is done by superusers using the following API call:

.. code-block:: bash
export SORT_ORDER=100
curl -X PUT -H 'Content-Type: application/json' -H X-Dataverse-key:$API_TOKEN $SERVER_URL/api/licenses/$ID/:sortOrder/$SORT_ORDER
.. _BagIt File Handler:

BagIt File Handler
Expand Down
3 changes: 2 additions & 1 deletion scripts/api/data/licenses/licenseCC-BY-4.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"uri": "http://creativecommons.org/licenses/by/4.0",
"shortDescription": "Creative Commons Attribution 4.0 International License.",
"iconUrl": "https://licensebuttons.net/l/by/4.0/88x31.png",
"active": true
"active": true,
"sortOrder": 2
}
3 changes: 2 additions & 1 deletion scripts/api/data/licenses/licenseCC-BY-NC-4.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"uri": "http://creativecommons.org/licenses/by-nc/4.0",
"shortDescription": "Creative Commons Attribution-NonCommercial 4.0 International License.",
"iconUrl": "https://licensebuttons.net/l/by-nc/4.0/88x31.png",
"active": true
"active": true,
"sortOrder": 4
}
3 changes: 2 additions & 1 deletion scripts/api/data/licenses/licenseCC-BY-NC-ND-4.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"uri": "http://creativecommons.org/licenses/by-nc-nd/4.0",
"shortDescription": "Creative Commons Attribution-NonCommercial-NoDerivatives 4.0 International License.",
"iconUrl": "https://licensebuttons.net/l/by-nc-nd/4.0/88x31.png",
"active": true
"active": true,
"sortOrder": 7
}
3 changes: 2 additions & 1 deletion scripts/api/data/licenses/licenseCC-BY-NC-SA-4.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"uri": "http://creativecommons.org/licenses/by-nc-sa/4.0",
"shortDescription": "Creative Commons Attribution-NonCommercial-ShareAlike 4.0 International License.",
"iconUrl": "https://licensebuttons.net/l/by-nc-sa/4.0/88x31.png",
"active": true
"active": true,
"sortOrder": 3
}
3 changes: 2 additions & 1 deletion scripts/api/data/licenses/licenseCC-BY-ND-4.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"uri": "http://creativecommons.org/licenses/by-nd/4.0",
"shortDescription": "Creative Commons Attribution-NoDerivatives 4.0 International License.",
"iconUrl": "https://licensebuttons.net/l/by-nd/4.0/88x31.png",
"active": true
"active": true,
"sortOrder": 6
}
3 changes: 2 additions & 1 deletion scripts/api/data/licenses/licenseCC-BY-SA-4.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"uri": "http://creativecommons.org/licenses/by-sa/4.0",
"shortDescription": "Creative Commons Attribution-ShareAlike 4.0 International License.",
"iconUrl": "https://licensebuttons.net/l/by-sa/4.0/88x31.png",
"active": true
"active": true,
"sortOrder": 5
}
3 changes: 2 additions & 1 deletion scripts/api/data/licenses/licenseCC0-1.0.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"uri": "http://creativecommons.org/publicdomain/zero/1.0",
"shortDescription": "Creative Commons CC0 1.0 Universal Public Domain Dedication.",
"iconUrl": "https://licensebuttons.net/p/zero/1.0/88x31.png",
"active": true
"active": true,
"sortOrder": 1
}
31 changes: 31 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/api/Licenses.java
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,37 @@ public Response setActiveState(@PathParam("id") long id, @PathParam("activeState
}
}

@PUT
@Path("/{id}/:sortOrder/{sortOrder}")
public Response setSortOrder(@PathParam("id") long id, @PathParam("sortOrder") long sortOrder) {
User authenticatedUser;
try {
authenticatedUser = findAuthenticatedUserOrDie();
if (!authenticatedUser.isSuperuser()) {
return error(Status.FORBIDDEN, "must be superuser");
}
} catch (WrappedResponse e) {
return error(Status.UNAUTHORIZED, "api key required");
}
try {
if (licenseSvc.setSortOrder(id, sortOrder) == 0) {
return error(Response.Status.NOT_FOUND, "License with ID " + id + " not found");
}
License license = licenseSvc.getById(id);
actionLogSvc
.log(new ActionLogRecord(ActionLogRecord.ActionType.Admin, "sortOrderLicenseChanged")
.setInfo("License " + license.getName() + "(" + license.getUri() + ") as id: " + id
+ "has now sort order " + sortOrder + ".")
.setUserIdentifier(authenticatedUser.getIdentifier()));
return ok("License ID " + id + " sort order set to " + sortOrder);
} catch (WrappedResponse e) {
if (e.getCause() instanceof IllegalArgumentException) {
return badRequest(e.getCause().getMessage());
}
return error(Response.Status.INTERNAL_SERVER_ERROR, e.getMessage());
}
}

@DELETE
@Path("/{id}")
public Response deleteLicenseById(@PathParam("id") long id) {
Expand Down
26 changes: 21 additions & 5 deletions src/main/java/edu/harvard/iq/dataverse/license/License.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
*/
@NamedQueries({
@NamedQuery( name="License.findAll",
query="SELECT l FROM License l ORDER BY (case when l.isDefault then 0 else 1 end), l.id asc"),
query="SELECT l FROM License l ORDER BY (case when l.isDefault then 0 else 1 end), l.sortOrder, l.id asc"),
@NamedQuery( name="License.findAllActive",
query="SELECT l FROM License l WHERE l.active='true' ORDER BY (case when l.isDefault then 0 else 1 end), l.id asc"),
query="SELECT l FROM License l WHERE l.active='true' ORDER BY (case when l.isDefault then 0 else 1 end), l.sortOrder, l.id asc"),
@NamedQuery( name="License.findById",
query = "SELECT l FROM License l WHERE l.id=:id"),
@NamedQuery( name="License.findDefault",
Expand All @@ -42,6 +42,8 @@
query = "UPDATE License l SET l.isDefault='false'"),
@NamedQuery( name="License.setActiveState",
query = "UPDATE License l SET l.active=:state WHERE l.id=:id"),
@NamedQuery( name="License.setSortOrder",
query = "UPDATE License l SET l.sortOrder=:sortOrder WHERE l.id=:id"),

})
@Entity
Expand Down Expand Up @@ -73,14 +75,17 @@ public class License {

@Column(nullable = false)
private boolean isDefault;

@Column(nullable = false, columnDefinition = "BIGINT NOT NULL DEFAULT 0")
private Long sortOrder;

@OneToMany(mappedBy="license")
private List<TermsOfUseAndAccess> termsOfUseAndAccess;

public License() {
}

public License(String name, String shortDescription, URI uri, URI iconUrl, boolean active) {
public License(String name, String shortDescription, URI uri, URI iconUrl, boolean active, Long sortOrder) {
this.name = name;
this.shortDescription = shortDescription;
this.uri = uri.toASCIIString();
Expand All @@ -91,6 +96,7 @@ public License(String name, String shortDescription, URI uri, URI iconUrl, boole
}
this.active = active;
isDefault = false;
this.sortOrder = sortOrder;
}

public Long getId() {
Expand Down Expand Up @@ -172,17 +178,26 @@ public void setTermsOfUseAndAccess(List<TermsOfUseAndAccess> termsOfUseAndAccess
this.termsOfUseAndAccess = termsOfUseAndAccess;
}

public Long getSortOrder() {
return sortOrder;
}

public void setSortOrder(Long sortOrder) {
this.sortOrder = sortOrder;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
License license = (License) o;
return active == license.active && id.equals(license.id) && name.equals(license.name) && shortDescription.equals(license.shortDescription) && uri.equals(license.uri) && Objects.equals(iconUrl, license.iconUrl);
return active == license.active && id.equals(license.id) && name.equals(license.name) && shortDescription.equals(license.shortDescription) && uri.equals(license.uri) && Objects.equals(iconUrl, license.iconUrl)
&& Objects.equals(sortOrder, license.sortOrder);
}

@Override
public int hashCode() {
return Objects.hash(id, name, shortDescription, uri, iconUrl, active);
return Objects.hash(id, name, shortDescription, uri, iconUrl, active, sortOrder);
}

@Override
Expand All @@ -195,6 +210,7 @@ public String toString() {
", iconUrl=" + iconUrl +
", active=" + active +
", isDefault=" + isDefault +
", sortOrder=" + sortOrder +
'}';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,23 @@ public int setActive(Long id, boolean state) throws WrappedResponse {
new IllegalArgumentException("License already " + (state ? "active" : "inactive")), null);
}
}

public int setSortOrder(Long id, Long sortOrder) throws WrappedResponse {
License candidate = getById(id);
if (candidate == null)
return 0;

return em.createNamedQuery("License.setSortOrder").setParameter("id", id).setParameter("sortOrder", sortOrder)
.executeUpdate();
}

public License save(License license) throws WrappedResponse {
if (license.getId() != null) {
throw new WrappedResponse(new IllegalArgumentException("There shouldn't be an ID in the request body"), null);
}
if (license.getSortOrder() == null) {
throw new WrappedResponse(new IllegalArgumentException("There should be a sort order value in the request body"), null);
}
try {
em.persist(license);
em.flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,8 @@ public static JsonObjectBuilder json(License license) {
.add("uri", license.getUri().toString())
.add("iconUrl", license.getIconUrl() == null ? null : license.getIconUrl().toString())
.add("active", license.isActive())
.add("isDefault", license.isDefault());
.add("isDefault", license.isDefault())
.add("sortOrder", license.getSortOrder());
}

public static Collector<String, JsonArrayBuilder, JsonArrayBuilder> stringsToJsonArray() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ALTER TABLE license
ADD COLUMN IF NOT EXISTS sortorder BIGINT NOT NULL DEFAULT 0;

CREATE INDEX IF NOT EXISTS license_sortorder_id
ON license (sortorder, id);
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ ALTER TABLE termsofuseandaccess ADD COLUMN IF NOT EXISTS license_id BIGINT;

DO $$
BEGIN

BEGIN
ALTER TABLE termsofuseandaccess ADD CONSTRAINT fk_termsofuseandcesss_license_id foreign key (license_id) REFERENCES license(id);
EXCEPTION
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public void testIsInReview() {
@Test
public void testGetJsonLd() throws ParseException {
Dataset dataset = new Dataset();
License license = new License("CC0 1.0", "You can copy, modify, distribute and perform the work, even for commercial purposes, all without asking permission.", URI.create("http://creativecommons.org/publicdomain/zero/1.0"), URI.create("/resources/images/cc0.png"), true);
License license = new License("CC0 1.0", "You can copy, modify, distribute and perform the work, even for commercial purposes, all without asking permission.", URI.create("http://creativecommons.org/publicdomain/zero/1.0"), URI.create("/resources/images/cc0.png"), true, 1l);
license.setDefault(true);
dataset.setProtocol("doi");
dataset.setAuthority("10.5072/FK2");
Expand Down
17 changes: 16 additions & 1 deletion src/test/java/edu/harvard/iq/dataverse/api/LicensesIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ public void testLicenses(){
getLicensesResponse.prettyPrint();
body = getLicensesResponse.getBody().asString();
status = JsonPath.from(body).getString("status");
long licenseId = JsonPath.from(body).getLong("data[-1].id");
//Last added license; with the highest id
long licenseId = (long) JsonPath.from(body).<Integer>getList("data.id").stream().max((x, y) -> Integer.compare(x, y)).get();
//Assumes the first license is active, which should be true on a test server
long activeLicenseId = JsonPath.from(body).getLong("data[0].id");
assertEquals("OK", status);
Expand Down Expand Up @@ -144,6 +145,20 @@ public void testLicenses(){
status = JsonPath.from(body).getString("status");
assertEquals("OK", status);

//Fail trying to set null sort order
Response setSortOrderErrorResponse = UtilIT.setLicenseSortOrderById(activeLicenseId, null, adminApiToken);
setSortOrderErrorResponse.prettyPrint();
body = setSortOrderErrorResponse.getBody().asString();
status = JsonPath.from(body).getString("status");
assertEquals("ERROR", status);

//Succeed in setting sort order
Response setSortOrderResponse = UtilIT.setLicenseSortOrderById(activeLicenseId, 2l, adminApiToken);
setSortOrderResponse.prettyPrint();
body = setSortOrderResponse.getBody().asString();
status = JsonPath.from(body).getString("status");
assertEquals("OK", status);

//Succeed in deleting our test license
Response deleteLicenseByIdResponse = UtilIT.deleteLicenseById(licenseId, adminApiToken);
deleteLicenseByIdResponse.prettyPrint();
Expand Down
9 changes: 8 additions & 1 deletion src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -2898,7 +2898,14 @@ static Response setLicenseActiveById(Long id, boolean state, String apiToken) {
.put("/api/licenses/"+id.toString() + "/:active/" + state);
return activateLicenseResponse;
}


static Response setLicenseSortOrderById(Long id, Long sortOrder, String apiToken) {
Response setSortOrderLicenseResponse = given()
.header(API_TOKEN_HTTP_HEADER, apiToken)
.urlEncodingEnabled(false)
.put("/api/licenses/"+id.toString() + "/:sortOrder/" + sortOrder);
return setSortOrderLicenseResponse;
}

static Response updateDatasetJsonLDMetadata(Integer datasetId, String apiToken, String jsonLDBody, boolean replace) {
Response response = given()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public static void tearDownClass() {
public void testExportDataset() throws Exception {
File datasetVersionJson = new File("src/test/resources/json/dataset-finch2.json");
String datasetVersionAsJson = new String(Files.readAllBytes(Paths.get(datasetVersionJson.getAbsolutePath())));
License license = new License("CC0 1.0", "You can copy, modify, distribute and perform the work, even for commercial purposes, all without asking permission.", URI.create("http://creativecommons.org/publicdomain/zero/1.0/"), URI.create("/resources/images/cc0.png"), true);
License license = new License("CC0 1.0", "You can copy, modify, distribute and perform the work, even for commercial purposes, all without asking permission.", URI.create("http://creativecommons.org/publicdomain/zero/1.0/"), URI.create("/resources/images/cc0.png"), true, 1l);
license.setDefault(true);

JsonReader jsonReader1 = Json.createReader(new StringReader(datasetVersionAsJson));
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/edu/harvard/iq/dataverse/util/FileUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public void testIsDownloadPopupRequiredLicenseCC0() {
DatasetVersion dsv1 = new DatasetVersion();
dsv1.setVersionState(DatasetVersion.VersionState.RELEASED);
TermsOfUseAndAccess termsOfUseAndAccess = new TermsOfUseAndAccess();
License license = new License("CC0", "You can copy, modify, distribute and perform the work, even for commercial purposes, all without asking permission.", URI.create("http://creativecommons.org/publicdomain/zero/1.0"), URI.create("/resources/images/cc0.png"), true);
License license = new License("CC0", "You can copy, modify, distribute and perform the work, even for commercial purposes, all without asking permission.", URI.create("http://creativecommons.org/publicdomain/zero/1.0"), URI.create("/resources/images/cc0.png"), true, 1l);
license.setDefault(true);
termsOfUseAndAccess.setLicense(license);
dsv1.setTermsOfUseAndAccess(termsOfUseAndAccess);
Expand All @@ -155,7 +155,7 @@ public void testIsDownloadPopupRequiredHasTermsOfUseAndCc0License() {
* the popup when the are Terms of Use. This feels like a bug since the
* Terms of Use should probably be shown.
*/
License license = new License("CC0", "You can copy, modify, distribute and perform the work, even for commercial purposes, all without asking permission.", URI.create("http://creativecommons.org/publicdomain/zero/1.0"), URI.create("/resources/images/cc0.png"), true);
License license = new License("CC0", "You can copy, modify, distribute and perform the work, even for commercial purposes, all without asking permission.", URI.create("http://creativecommons.org/publicdomain/zero/1.0"), URI.create("/resources/images/cc0.png"), true, 2l);
license.setDefault(true);
termsOfUseAndAccess.setLicense(license);
termsOfUseAndAccess.setTermsOfUse("be excellent to each other");
Expand Down
3 changes: 2 additions & 1 deletion src/test/resources/json/license.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"uri": "http://dataverse..org/licenses/test/1.0",
"iconUrl": "http://dataverse.org/licenses/test/1.0/icon.png",
"shortDescription": "Dataverse Test License v1.0.",
"active": false
"active": false,
"sortOrder": 1000
}
3 changes: 2 additions & 1 deletion src/test/resources/json/licenseError.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"uri": "http://dataverse..org/licenses/test/ln6",
"iconUrl": "http://dataverse.org/licenses/test/ln6/icon.png",
"shortDescription": "A License that must have id 6.",
"active": true
"active": true,
"sortOrder": 1
}

0 comments on commit 948c0d0

Please sign in to comment.