blocked deposit page, hide host field when only one collection (e.g. root) #11301
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ofahimIQSS
moved this from Ready for Triage
to Ready for Review ⏩
in IQSS Dataverse Project
|
Can you reproduce this on demo (which is v6.5)? The code changes look OK but I haven't yet been able to trigger the problem on demo.dataverse.org. |
|
I believe I was able to reproduce the bug on ab8110f (the latest in develop). The page is greyed out like this:
Stacktrace: |
|
I tried again and I was able to reproduce it again. One difference for me, perhaps, is that after a hard-reset the fields I entered are still there, with the exception of the Subject field. So I don't lose all of my work, but still, it's a very annoying user experience! |
|
@jo-pol I switched to this branch/PR and the stacktrace looks different but I have the same behavior in the UI. The screen goes gray-ish and the dataset isn't saved.
|
|
FWIW: Looks like it only occurs when there are no templates for some reason. |
|
@pdurbin |
|
Tried the IQSS develop branch on a DANS v6.5 VM and got an internal DB error. |
|
@jo-pol sorry, I was trying to use the same terminology you were. I said "hard reset" when I meant "hard refresh". What was the internal DB error? 🤔 |
|
The DB error was |
|
Tried returning null in stead of a string also worked on a DANS v6.5 VM. Examining the stack trace and corresponding jakarta/faces code that would cause a null pointer exception on IQSS development. So now I used zero as default. Don't know about the effects on templates and/or when the initial dataverse was another one. |
|
Not sure but I might have stayed on the page https://dev.archaeology.datastations.nl/dataset.xhtml?ownerId=1 when redeploying the war when I saw the DB error. |
Right, sorry, that was our bad. Fixed in this PR: |
cmbz
added
FY25 Sprint 19
|
@jo-pol we just released Dataverse 6.6. Can you please merge the latest from develop into this branch? Also, as I said above, I can definitely reproduce some strangeness but this pull request doesn't seem to help (the error in server.log was different). Am I missing something? Thanks! |
|
@pdurbin |
pdurbin
changed the title
|
@jo-pol I just tried and no, I was testing an earlier commit last time. This time I'm on the latest (95860a6) and I couldn't reproduce the bug! 🎉 Perfect! 🚀 I noticed that you added a hasDataversesToChoose boolean. I took the liberty of updating the title of this pull request to reflect this. Under "Does this PR introduce a user interface change?" can you please upload "before" and "after" screenshots or at least describe the change? Do you want this hasDataversesToChoose behavior to continue in the new React UI? If so, please open an issue at https://github.com/IQSS/dataverse-frontend Finally, could you please also add a release note snippet to this PR? You could call it p.s. I noticed API tests are failing but it was a problem with Jenkins/AWS so I started another build. Since this is a frontend change, it shouldn't affect API tests. |
github-project-automation
bot
moved this from In Review 🔎
to Ready for QA ⏩
in IQSS Dataverse Project
|
I'm getting a 500 error page when I push this to internal: |
|
@ofahimIQSS what do you see in server.log? |
qqmyers
reviewed
Apr 11, 2025
| @@ -26,7 +26,9 @@ public class DataverseConverter implements Converter { | |||
|
|
|||
| @Override | |||
| public Object getAsObject(FacesContext facesContext, UIComponent component, String submittedValue) { | |||
| return dataverseService.find(new Long(submittedValue)); | |||
| var pk = (submittedValue == null || submittedValue.isEmpty() || submittedValue.matches(".*[^0-9].*")) | |||
| ? 0 : Long.valueOf(submittedValue); | |||
There was a problem hiding this comment.
FWIW - the root dataverse does not always have id=0
There was a problem hiding this comment.
FWIW: There is a findRoot call that could be used instead.
There was a problem hiding this comment.
@jo-pol can you please switch this to dataverseService.findRootDataverse() instead? Thanks.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
A dataset is not saved and there is no option to correct the mistake if you edit the “Host Dataverse” field and immediately correct and leave it. When clicking Save the page freezes and after a hard refresh all your metadata is gone and you have to start over. Reproduced in 6.3 and 6.5 (DANS branches).
A more robust solution is not to show the edit field at al when there is nothing to choose.
Which issue(s) this PR closes:
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?:
Perhaps: the host dataverse field does not appear when there are none to choose from.
Additional documentation: