Skip to content

Comments

Remove hard-coded English strings from MyData API#9786

Merged
sekmiller merged 9 commits intoIQSS:developfrom
Recherche-Data-Gouv:9292-my-data-hardcoded-text
May 22, 2024
Merged

Remove hard-coded English strings from MyData API#9786
sekmiller merged 9 commits intoIQSS:developfrom
Recherche-Data-Gouv:9292-my-data-hardcoded-text

Conversation

@stevenferey
Copy link
Contributor

What this PR does / why we need it:

Which issue(s) this PR closes:

Closes #9292

Special notes for your reviewer:

Suggestions on how to test this:

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?:

Additional documentation:

Copy link
Member

@qqmyers qqmyers left a comment

Choose a reason for hiding this comment

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

Looks good - did not test but the auto-test and QA should be able to verify the messages aren't getting changed.
FWIW: There are a couple weird text strings in the original that this PR faithfully makes internationalizable, i.e. "retrieveMyDataAsJsonString. User not found! Shouldn't be using this anyway.", "Sorry, the EntityManager isn't working (still).". If anyone knows more about when they might appear, perhaps some more user-oriented text could be substituted.

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 didn't test this but it makes sense. Thanks, @stevenferey! We're in the middle of a platform upgrade only release (6.0), but after that we can try to get this in. I love it when we removed hard-coded English.

myDataFilterParams.error.result.no.role=No results. Please select at least one Role.
myDataFilterParams.error.result.no.dvobject=No results. Please select one of Dataverses, Datasets, Files.
myDataFilterParams.error.result.no.publicationStatus=No results. Please select one of {0}.
myDataFinder.error.result.null=Sorry, the EntityManager isn't working (still).
Copy link
Member

Choose a reason for hiding this comment

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

Ha. The poor translators. What in the world is an EntityManager, they'll wonder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A little more contextualization in this case with: "Sorry, the authenticated user ID could not be retrieved."

@pdurbin pdurbin added Feature: Internationalization Size: 3 A percentage of a sprint. 2.1 hours. labels Aug 17, 2023
@pdurbin pdurbin changed the title 9292 my data hardcoded text Remove hard-coded English strings from MyData API Feb 28, 2024
@pdurbin pdurbin changed the title Remove hard-coded English strings from MyData API 9292 my data hardcoded text Feb 28, 2024
@pdurbin pdurbin added the Type: Bug a defect label Feb 28, 2024
@pdurbin pdurbin changed the title 9292 my data hardcoded text Remove hard-coded English strings from MyData API Feb 28, 2024
Copy link
Member

@qqmyers qqmyers left a comment

Choose a reason for hiding this comment

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

@stevenferey - looks good - probably needs a merge with develop to get the tests to run again.

#mydata API (DataRetriverAPI.java and MyDataFinder.java)
dataretrieverAPI.noMsgResultsFound=Sorry, no results were found.
dataretrieverAPI.authentication.required=Requires authentication. Please login.
dataretrieverAPI.authentication.required.opt=retrieveMyDataAsJsonString. User not found! Shouldn't be using this anyway.
Copy link
Member

Choose a reason for hiding this comment

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

FWIW: This is still odd, but only shows in the log.

@sekmiller
Copy link
Contributor

@stevenferey This PR has a merge conflict. Please resolve. Also I'm seeing a failing test in DataRetrieverApiIT. Can you please take a look at that? Thanks!

@sekmiller sekmiller added the Status: Needs Input Applied to issues in need of input from someone currently unavailable label May 13, 2024
@coveralls
Copy link

coveralls commented May 15, 2024

Coverage Status

coverage: 20.586% (-0.004%) from 20.59%
when pulling 36379c6 on Recherche-Data-Gouv:9292-my-data-hardcoded-text
into da3dd95 on IQSS:develop.

@sekmiller sekmiller removed the Status: Needs Input Applied to issues in need of input from someone currently unavailable label May 15, 2024
@sekmiller
Copy link
Contributor

sekmiller commented May 16, 2024

`
private static final String ERR_MSG_FORMAT = "{\n "success":false,\n "error_message": "%s"\n}";

    assertEquals(prettyPrintError("dataretrieverAPI.user.not.found", Arrays.asList(badUserIdentifier)).replace(" ", ""), invalidUserIdentifierResponse.prettyPrint().replace(" ", ""));


    assertEquals(prettyPrintError("myDataFinder.error.result.no.role", null).replace(" ", ""), validUserIdentifierResponse.prettyPrint().replace(" ", ""));

`

@stevenferey Tests are failing - I was able to get them to pass by making these changes locally. I'm not able to update your repo. Does this make sense? And, if so, can you make the changes. Thanks!

@sekmiller sekmiller added the Status: Needs Input Applied to issues in need of input from someone currently unavailable label May 21, 2024
@stevenferey
Copy link
Contributor Author

Thank you for this feedback,

We have updated the error message format in the test case.

@sekmiller sekmiller removed the Status: Needs Input Applied to issues in need of input from someone currently unavailable label May 22, 2024
@sekmiller
Copy link
Contributor

Looks good @stevenferey @jeromeroucou

Thanks for the contribution and your patience through the process

@sekmiller sekmiller merged commit 55fb1bd into IQSS:develop May 22, 2024
@pdurbin pdurbin added this to the 6.3 milestone May 23, 2024
@luddaniel luddaniel deleted the 9292-my-data-hardcoded-text branch September 19, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature: Internationalization Size: 3 A percentage of a sprint. 2.1 hours. Type: Bug a defect

Projects

Status: 🚀 Done (Recherche Data Gouv)

Development

Successfully merging this pull request may close these issues.

Translate hard coded text - My Data page

6 participants