Skip to content

Conversation

@jp-tosca
Copy link
Contributor

@jp-tosca jp-tosca commented May 15, 2024

What this PR does / why we need it:
After debugging this test and the endpoint to resolve the issues on #10312 I made a few changes:

  1. This test was relying heavily on having any of the specified locales in the JSON but with no reason. This dependency was cause due the inability of the API to return the ID of the created banner and the need to search the id by the display message, see this comment. To avoid this, the endpoint api/admin/bannerMessage has been extended so the ID is returned when created, this will allow to delete a specific banner with ID after creation. This is not a breaking change since is not removing anything from the JSON.
  2. The method UtilIT.getBannerMessageIdFromResponse is not necessary anymore, it also was hardcoded a return to 0 if the string was not found but the id could be some other number.

Which issue(s) this PR closes:

Closes #10312

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

@coveralls
Copy link

coveralls commented May 15, 2024

Coverage Status

coverage: 20.657% (+0.08%) from 20.574%
when pulling a47329f on rocky_IX
into 5bf6b6d on develop.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

import jakarta.ws.rs.core.Response.Status;

import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.exception.ExceptionUtils;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.IllegalImportCheck> reported by reviewdog 🐶
Illegal import - org.apache.commons.lang.exception.ExceptionUtils.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@jp-tosca
Copy link
Contributor Author

With the last changes I updated the endpoint api/admin/bannerMessage this test will still fail if no locale is defined but this time will be at the API level not at the Test level. This endpoint will return a 500 with No banner messages found for this locale. if there is no banner for this locale over an array with empty elements.

@github-actions

This comment has been minimized.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a quick look.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc change looks good. I left comments on the Makefile.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left another comment on the Makefile.

@github-actions

This comment has been minimized.

@jp-tosca
Copy link
Contributor Author

I had to change the make docs-all and docs-pdf since I didn't noticed these requires sphinx-latexpdf over sphinx. They work fine now.

@github-actions

This comment has been minimized.

@sekmiller sekmiller self-assigned this Jun 11, 2024
@github-actions
Copy link

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:rocky-IX
ghcr.io/gdcc/configbaker:rocky-IX

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

@sekmiller sekmiller removed their assignment Jun 14, 2024
@stevenwinship stevenwinship self-assigned this Jun 18, 2024
@github-actions
Copy link

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:rocky-IX
ghcr.io/gdcc/configbaker:rocky-IX

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

@stevenwinship stevenwinship merged commit 853965e into develop Jun 18, 2024
@stevenwinship stevenwinship deleted the rocky_IX branch June 18, 2024 19:19
@stevenwinship stevenwinship removed their assignment Jun 18, 2024
pdurbin added a commit to pdurbin/dataverse that referenced this pull request Jul 1, 2024
These changes were in 853965e as part of PR IQSS#10565 but were lost during
resolution of a merge conflict.
@pdurbin pdurbin added this to the 6.3 milestone Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: 30 A percentage of a sprint. 21 hours. (formerly size:33)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AdminIT.testBannerMessages failing on Rocky 9

6 participants