Skip to content

Conversation

@qqmyers
Copy link
Member

@qqmyers qqmyers commented Mar 13, 2025

What this PR does / why we need it: Addresses the bugs and performance issues found manually after the merge of #11312. Specific changes include:

  • Reverting DatasetFieldType displayOnCreate to be boolean (non-nullable) - not needed
  • Changing DataverseFieldTypeInputLevel dsiplayOnCreate to be Boolean (nullable) - allows displayOnCreate to be optional in the Dataverse API, with default back to the DatasetFieldType value
  • Change in the JsonPrinter logic to pick up the correct values from the inputlevel or type as appropriate - both w.r.t. whether to include fields and the values included when adding the types to the response.
  • Update to the MetadataBlockServiceBean query to find displayOnCreate blocks to improve performance (from @GPortas)

Which issue(s) this PR closes:

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

Additional documentation:

qqmyers added 16 commits March 11, 2025 14:39
'IQSS/10476-display-on-create-field-option-new' into
10476-display-on-create-field-option-new
'IQSS/10476-display-on-create-field-option-new' into
10476-display-on-create-field-option-new
'IQSS/10476-display-on-create-field-option-new' into
10476-display-on-create-field-option-new
'IQSS/10476-display-on-create-field-option-new' into
10476-display-on-create-field-option-new
@coveralls
Copy link

coveralls commented Mar 13, 2025

Coverage Status

coverage: 22.596%. remained the same
when pulling d66ca3b on GlobalDataverseCommunityConsortium:10476-display-on-create-field-option-new
into 1480dbc on IQSS:develop.

@qqmyers qqmyers marked this pull request as ready for review March 17, 2025 12:05
@qqmyers qqmyers moved this to Ready for Review ⏩ in IQSS Dataverse Project Mar 17, 2025
@qqmyers qqmyers added this to the 6.6 milestone Mar 17, 2025
@qqmyers
Copy link
Member Author

qqmyers commented Mar 17, 2025

Bad force-push caused a close

@qqmyers qqmyers reopened this Mar 17, 2025
@GPortas GPortas self-assigned this Mar 17, 2025
@GPortas GPortas moved this from Ready for Review ⏩ to In Review 🔎 in IQSS Dataverse Project Mar 17, 2025
@Saixel Saixel requested review from GPortas, pdurbin and sekmiller March 17, 2025 15:13
Copy link
Contributor

@GPortas GPortas left a comment

Choose a reason for hiding this comment

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

It looks great overall—just a few minor points that would be nice to clarify, but given the priority, it's ready for approval.

Copy link
Contributor

@GPortas GPortas 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 to me!

@github-project-automation github-project-automation bot moved this from In Review 🔎 to Ready for QA ⏩ in IQSS Dataverse Project Mar 17, 2025
@GPortas GPortas removed their assignment Mar 17, 2025
@qqmyers qqmyers removed their assignment Mar 17, 2025
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 wasn't able to reproduce the bugs I've seen on develop:

  • I can see all the fields I expect
  • performance is better

@ofahimIQSS ofahimIQSS self-assigned this Mar 17, 2025
@ofahimIQSS ofahimIQSS moved this from Ready for QA ⏩ to QA ✅ in IQSS Dataverse Project Mar 17, 2025
@ofahimIQSS
Copy link
Contributor

I pushed this PR to internal and tried creating a dataset and noticed that some fields were missing i.e., keyword, related publication - citation block, and notes.

Screen.Recording.2025-03-17.at.3.10.54.PM.mov

@qqmyers
Copy link
Member Author

qqmyers commented Mar 17, 2025

Guessing this is issues on internal - I see that the notes fieldtype has required = false there - probably from earlier testing. Re loading the citation /other blocks may help.

@ofahimIQSS
Copy link
Contributor

Thanks @qqmyers - after I reloaded the metadata block in internal, I was able to see the fields that were initially missing.

@ofahimIQSS
Copy link
Contributor

ofahimIQSS commented Mar 17, 2025

looks good from my end. Merging PR

image

@ofahimIQSS ofahimIQSS merged commit b0d9ab0 into IQSS:develop Mar 17, 2025
11 checks passed
@github-project-automation github-project-automation bot moved this from QA ✅ to Merged 🚀 in IQSS Dataverse Project Mar 17, 2025
@ofahimIQSS ofahimIQSS removed their assignment Mar 17, 2025
@cmbz cmbz added the FY25 Sprint 19 FY25 Sprint 19 (2025-03-12 - 2025-03-26) label Mar 17, 2025
@scolapasta scolapasta moved this from Merged 🚀 to Done 🧹 in IQSS Dataverse Project Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FY25 Sprint 19 FY25 Sprint 19 (2025-03-12 - 2025-03-26) Size: 10 A percentage of a sprint. 7 hours.

Projects

Status: Done 🧹

Development

Successfully merging this pull request may close these issues.

6 participants