Banner IT improvements after resolving issues on rocky. (#10565)

jp-tosca · web-flow · commit 853965ec6058 · 2024-06-18T15:19:44.000-04:00
* Rocky IX

* Logging pretty print

* Removed use of function with hardcoded valie

* Clearing banners

* Added codeblock with JSON

* Banners size

* Trace added

* Import fix

* Pull test

* lOG TEST

* Log test

* Log test

* ASD

* Debug commit

* Debug commit

* Dbug commit

* Dbug commit

* Bbug commit

* Dbug cmmt

* dbg cmmt

* dbg cmmit

* Dbg cmmt

* reset bannermessage

* Reset Admin

* Reset AdminIT

* Reset admin IT

* Clear console output

* Last changes

* Last changes for this test

* Last update

* Locale check

* Docs and makefile to build guides

* Makefile changes

* Value of sphinx version loaded from requirements.txt

* Last changes to makefile and docs

* Correction to the docs
diff --git a/doc/release-notes/10565-banner-test-improvements.md b/doc/release-notes/10565-banner-test-improvements.md
@@ -0,0 +1 @@
+The endpoint `api/admin/bannerMessage` has been extended so the ID is returned when created
diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst
@@ -5194,6 +5194,8 @@ Delete the setting under ``name``::
 Manage Banner Messages
 ~~~~~~~~~~~~~~~~~~~~~~
 
+.. warning:: Adding a banner message with a language that is not supported by the installation will result in a 500-Internal Server Error response when trying to access to the /bannerMessage.
+
 Communications to users can be handled via banner messages that are displayed at the top of all pages within your Dataverse installation. Two types of banners can be configured:
 
 - A banner message where dismissibleByUser is set to false will be displayed to anyone viewing the application. These messages will be dismissible for a given session but will be displayed in any subsequent session until they are deleted by the Admin. This type of banner message is useful for situations such as upcoming maintenance windows and other downtime.
diff --git a/doc/sphinx-guides/source/developers/documentation.rst b/doc/sphinx-guides/source/developers/documentation.rst
@@ -92,9 +92,20 @@ Open a terminal, change directories to ``doc/sphinx-guides``, activate (or react
 Building the Guides with a Sphinx Docker Container
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+**Building with Docker and Makefile:**
+
+We have added a Makefile to simplify the process of building the guides using a Docker container, you can use some of the following from the repository root:
+
+- `make docs-html`
+- `make docs-pdf`
+- `make docs-epub`
+- `make docs-all`
+
+**Building with Docker and CLI:**
+
 If you want to build the guides using a Docker container, execute the following command in the repository root:
 
-``docker run -it --rm -v $(pwd):/docs sphinxdoc/sphinx:3.5.4 bash -c "cd doc/sphinx-guides && pip3 install -r requirements.txt && make html"``
+``docker run -it --rm -v $(pwd):/docs sphinxdoc/sphinx:7.2.6 bash -c "cd doc/sphinx-guides && pip3 install -r requirements.txt && make html"``
 
 Previewing the Guides
 ^^^^^^^^^^^^^^^^^^^^^
@@ -148,7 +159,7 @@ The HTML version of the guides is the official one. Any other formats are mainta
 
 If you would like to build a PDF version of the guides and have Docker installed, please try the command below from the root of the git repo:
 
-``docker run -it --rm -v $(pwd):/docs sphinxdoc/sphinx-latexpdf:3.5.4 bash -c "cd doc/sphinx-guides && pip3 install -r requirements.txt && make latexpdf LATEXMKOPTS=\"-interaction=nonstopmode\"; cd ../.. && ls -1 doc/sphinx-guides/build/latex/Dataverse.pdf"``
+``docker run -it --rm -v $(pwd):/docs sphinxdoc/sphinx-latexpdf:7.2.6 bash -c "cd doc/sphinx-guides && pip3 install -r requirements.txt && make latexpdf LATEXMKOPTS=\"-interaction=nonstopmode\"; cd ../.. && ls -1 doc/sphinx-guides/build/latex/Dataverse.pdf"``
 
 A few notes about the command above:
 
diff --git a/makefile b/makefile
@@ -0,0 +1,14 @@
+
+SPHINX_VERSION = $(shell grep "Sphinx" ./doc/sphinx-guides/requirements.txt | awk -F'==' '{print $$2}')
+docs-html:
+	docker run -it --rm -v $$(pwd):/docs sphinxdoc/sphinx:$(SPHINX_VERSION) bash -c "cd doc/sphinx-guides && pip3 install -r requirements.txt && make clean && make html"
+
+docs-pdf:
+	docker run -it --rm -v $$(pwd):/docs sphinxdoc/sphinx-latexpdf:$(SPHINX_VERSION) bash -c "cd doc/sphinx-guides && pip3 install -r requirements.txt && make clean && make latexpdf LATEXMKOPTS=\"-interaction=nonstopmode\"; cd ../.. && ls -1 doc/sphinx-guides/build/latex/Dataverse.pdf"
+
+docs-epub:
+	docker run -it --rm -v $$(pwd):/docs sphinxdoc/sphinx:$(SPHINX_VERSION) bash -c "cd doc/sphinx-guides && pip3 install -r requirements.txt && make clean && make epub"
+
+docs-all:
+	docker run -it --rm -v $$(pwd):/docs sphinxdoc/sphinx:$(SPHINX_VERSION) bash -c "cd doc/sphinx-guides && pip3 install -r requirements.txt && make clean && make html && make epub"
+	docker run -it --rm -v $$(pwd):/docs sphinxdoc/sphinx-latexpdf:$(SPHINX_VERSION) bash -c "cd doc/sphinx-guides && pip3 install -r requirements.txt && make latexpdf LATEXMKOPTS=\"-interaction=nonstopmode\"; cd ../.. && ls -1 doc/sphinx-guides/build/latex/Dataverse.pdf"
diff --git a/src/main/java/edu/harvard/iq/dataverse/BannerMessage.java b/src/main/java/edu/harvard/iq/dataverse/BannerMessage.java
@@ -46,7 +46,7 @@ public void setBannerMessageTexts(Collection<BannerMessageText> bannerMessageTex
     
     
     public String getDisplayValue(){        
-        String retVal = "";
+        String retVal = null;
         for (BannerMessageText msgTxt : this.getBannerMessageTexts()) {
             if (msgTxt.getLang().equals(BundleUtil.getCurrentLocale().getLanguage())) {
                 retVal = msgTxt.getMessage();
diff --git a/src/main/java/edu/harvard/iq/dataverse/BannerMessageServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/BannerMessageServiceBean.java
@@ -46,8 +46,10 @@ public List<BannerMessage> findAllBannerMessages() {
     
     public void save( BannerMessage message ) {
         em.persist(message);
+        em.flush();
     }
     
+    
     public void deleteBannerMessage(Object pk) {
         BannerMessage message = em.find(BannerMessage.class, pk);
 
diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java
@@ -2357,8 +2357,13 @@ public Response addBannerMessage(JsonObject jsonObject) throws WrappedResponse {
                 messageText.setBannerMessage(toAdd);
                 toAdd.getBannerMessageTexts().add(messageText);
             }
-                bannerMessageService.save(toAdd);
-                return ok("Banner Message added successfully.");
+            bannerMessageService.save(toAdd);
+
+            JsonObjectBuilder jsonObjectBuilder = Json.createObjectBuilder()
+                .add("message", "Banner Message added successfully.")
+                .add("id", toAdd.getId());
+
+            return ok(jsonObjectBuilder);
 
         } catch (Exception e) {
             logger.warning("Unexpected Exception: " + e.getMessage());
@@ -2398,10 +2403,19 @@ public Response deactivateBannerMessage(@PathParam("id") Long id) throws Wrapped
     @Path("/bannerMessage")
     public Response getBannerMessages(@PathParam("id") Long id) throws WrappedResponse {
 
-        return ok(bannerMessageService.findAllBannerMessages().stream()
-                .map(m -> jsonObjectBuilder().add("id", m.getId()).add("displayValue", m.getDisplayValue()))
-                .collect(toJsonArray()));
+        List<BannerMessage> messagesList = bannerMessageService.findAllBannerMessages();
+
+        for (BannerMessage message : messagesList) {
+            if ("".equals(message.getDisplayValue())) {
+               return error(Response.Status.INTERNAL_SERVER_ERROR, "No banner messages found for this locale.");
+            }
+        }
 
+        JsonArrayBuilder messages = messagesList.stream()
+        .map(m -> jsonObjectBuilder().add("id", m.getId()).add("displayValue", m.getDisplayValue()))
+        .collect(toJsonArray());
+        
+        return ok(messages);
     }
     
     @POST
diff --git a/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java b/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java
@@ -15,22 +15,21 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
-import static jakarta.ws.rs.core.Response.Status.FORBIDDEN;
-import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST;
 
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
+
+
+
 import java.util.Map;
 import java.util.UUID;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import static jakarta.ws.rs.core.Response.Status.CREATED;
-import static jakarta.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR;
-import static jakarta.ws.rs.core.Response.Status.OK;
-import static jakarta.ws.rs.core.Response.Status.UNAUTHORIZED;
+import static jakarta.ws.rs.core.Response.Status.*;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.notNullValue;
@@ -832,36 +831,47 @@ public void testClearThumbnailFailureFlag(){
     
     @Test
     public void testBannerMessages(){
-        
-        String pathToJsonFile = "scripts/api/data/bannerMessageError.json";
-        Response addBannerMessageErrorResponse = UtilIT.addBannerMessage(pathToJsonFile);
+
+        //We check for existing banner messages and get the number of existing messages
+        Response getBannerMessageResponse = UtilIT.getBannerMessages();
+        getBannerMessageResponse.prettyPrint();
+        getBannerMessageResponse.then().assertThat()
+                .statusCode(OK.getStatusCode());
+        Integer numBannerMessages =
+                JsonPath.from(getBannerMessageResponse.getBody().asString()).getInt("data.size()");
+
+        //We add a banner message with an error in the json file
+        String pathToJsonFile = "scripts/api/data/bannerMessageError.json";       
+        Response addBannerMessageErrorResponse  = UtilIT.addBannerMessage(pathToJsonFile);
         addBannerMessageErrorResponse.prettyPrint();
-        String body = addBannerMessageErrorResponse.getBody().asString();
-        String status = JsonPath.from(body).getString("status");
-        assertEquals("ERROR", status);
+        addBannerMessageErrorResponse.then().assertThat()
+                        .statusCode(BAD_REQUEST.getStatusCode())
+                        .body("status", equalTo("ERROR"));
         
+        //We add a banner message with a correct json file
         pathToJsonFile = "scripts/api/data/bannerMessageTest.json";
-        
         Response addBannerMessageResponse = UtilIT.addBannerMessage(pathToJsonFile);
         addBannerMessageResponse.prettyPrint();
-        body = addBannerMessageResponse.getBody().asString();
-        status = JsonPath.from(body).getString("status");
-        assertEquals("OK", status);
+        addBannerMessageResponse.then().assertThat()
+                .statusCode(OK.getStatusCode())
+                .body("status", equalTo("OK"))
+                .body("data.message", equalTo("Banner Message added successfully."));
+        Long addedBanner = Long.valueOf(
+                        JsonPath.from(addBannerMessageResponse.getBody().asString()).getLong("data.id"));                
         
-        Response getBannerMessageResponse = UtilIT.getBannerMessages();
+        //We get the banner messages and check that the number of messages has increased by 1
+        getBannerMessageResponse = UtilIT.getBannerMessages();
         getBannerMessageResponse.prettyPrint();
-        body = getBannerMessageResponse.getBody().asString();
-        status = JsonPath.from(body).getString("status");
-        assertEquals("OK", status);
-        String deleteId = UtilIT.getBannerMessageIdFromResponse(getBannerMessageResponse.getBody().asString());
-         
-        System.out.print("delete id: " + deleteId);
-        
-        Response deleteBannerMessageResponse = UtilIT.deleteBannerMessage(new Long (deleteId));
+        getBannerMessageResponse.then().assertThat()
+                .statusCode(OK.getStatusCode())
+                .body("data.size()", equalTo(numBannerMessages + 1));
+
+        //We delete the banner message
+        Response deleteBannerMessageResponse = UtilIT.deleteBannerMessage(addedBanner);
         deleteBannerMessageResponse.prettyPrint();
-        body = deleteBannerMessageResponse.getBody().asString();
-        status = JsonPath.from(body).getString("status");
-        assertEquals("OK", status);
+        deleteBannerMessageResponse.then().assertThat()
+                .statusCode(OK.getStatusCode())
+                .body("status", equalTo("OK"));
         
     }
 
diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java
@@ -3387,20 +3387,6 @@ static Response deleteBannerMessage(Long id) {
         return deleteBannerMessageResponse;
     }
     
-    static String getBannerMessageIdFromResponse(String getBannerMessagesResponse) {
-        StringReader rdr = new StringReader(getBannerMessagesResponse);
-        JsonObject json = Json.createReader(rdr).readObject();
-
-        for (JsonObject obj : json.getJsonArray("data").getValuesAs(JsonObject.class)) {
-            String message = obj.getString("displayValue");
-            if (message.equals("Banner Message For Deletion")) {
-                return obj.getJsonNumber("id").toString();
-            }
-        }
-
-        return "0";
-    }
-    
     static Response getDatasetJsonLDMetadata(Integer datasetId, String apiToken) {
         Response response = given()
                 .header(API_TOKEN_HTTP_HEADER, apiToken)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+The endpoint `api/admin/bannerMessage` has been extended so the ID is returned when created
Original file line number	Diff line number	Diff line change
`@@ -46,8 +46,10 @@ public List<BannerMessage> findAllBannerMessages() {`
`46`	`46`
`47`	`47`	`public void save( BannerMessage message ) {`
`48`	`48`	`em.persist(message);`
	`49`	`+ em.flush();`
`49`	`50`	`}`
`50`	`51`
	`52`	`+`
`51`	`53`	`public void deleteBannerMessage(Object pk) {`
`52`	`54`	`BannerMessage message = em.find(BannerMessage.class, pk);`
`53`	`55`