Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ListMetadataBlocksCommand to not return fields marked as displayOnCreate=true if there is an input level with include=false #10741

Closed
GPortas opened this issue Aug 5, 2024 · 0 comments · Fixed by #10767
Assignees
Labels
GREI Re-arch Issues related to the GREI Dataverse rearchitecture Original size: 10 Size: 10 A percentage of a sprint. 7 hours. SPA.Q3 Not related to any specific Q3 feature SPA These changes are required for the Dataverse SPA Type: Bug a defect
Milestone

Comments

@GPortas
Copy link
Contributor

GPortas commented Aug 5, 2024

What steps does it take to reproduce the issue?

Create a collection from JSF, in the metadata blocks section, uncheck the option to inherit the metadata blocks from the parent. In citation, deselect the Notes field, which is a field with displayOnCreate=true in the database.

Submit the new collection. When creating a dataset in the new collection, you will see that Notes does not appear, even though the configuration will remain with displayOnCreate=true in the database. The input level with include=false takes precedence over displayOnCreate=true and therefore is not displayed.

The API for collection/{id}/metadata blocks, which uses ListMetadataBlocksCommand, returns this field when displayOnCreate=true.

Since there is an input level with include=false, this field should not be returned from the API.

We need to update the related queries in MetadataBlockServiceBean

  • When does this issue occur?

We detected it on the SPA, since it uses the mentioned endpoint.

  • Which page(s) does it occurs on?

None on JSF

  • What happens?

n/a

  • To whom does it occur (all users, curators, superusers)?

API users

  • What did you expect to happen?

The API for collection/{id}/metadata blocks, which uses ListMetadataBlocksCommand, should not any field with displayOnCreate=true if there is an input level with include=false.

Which version of Dataverse are you using?

latest develop

Any related open or closed issues to this bug report?

@GPortas GPortas added Type: Bug a defect Size: 10 A percentage of a sprint. 7 hours. SPA These changes are required for the Dataverse SPA GREI Re-arch Issues related to the GREI Dataverse rearchitecture labels Aug 5, 2024
@GPortas GPortas changed the title Fix ListMetadataBlocksCommand to not return fields marked as displayOnCreate=true if there is an input levels with include=false Fix ListMetadataBlocksCommand to not return fields marked as displayOnCreate=true if there is an input level with include=false Aug 5, 2024
@GPortas GPortas moved this to This Sprint 🏃‍♀️ 🏃 in IQSS Dataverse Project Aug 8, 2024
@GPortas GPortas self-assigned this Aug 8, 2024
GPortas added a commit that referenced this issue Aug 13, 2024
stevenwinship pushed a commit that referenced this issue Aug 20, 2024
…lds marked as displayOnCreate=true if there is an input level with include=false (#10767)

* Fixed: MetadataBlockServiceBean to check for not excluded fields in input levels

* Changed: using queries for obtaining dataset field types based on displaying conditions

* Refactor: json printer method for MetadataBlock

* Added: IT test case for list metadata blocks testing field with include=false and displayOnCreate=true property

* Fixed: removed condition in MetadataBlockServiceBean

* Added: release notes for #10741

* Fixed: displayOnCreate query logic

* Fixed: excluding conditionally required fields when display-on-create is true

* Fixed: query predicate for required-in-dataverse field condition
@pdurbin pdurbin added this to the 6.4 milestone Aug 23, 2024
stevenwinship pushed a commit that referenced this issue Sep 4, 2024
* Fixed: MetadataBlockServiceBean to check for not excluded fields in input levels

* Changed: using queries for obtaining dataset field types based on displaying conditions

* Refactor: json printer method for MetadataBlock

* Added: IT test case for list metadata blocks testing field with include=false and displayOnCreate=true property

* Fixed: removed condition in MetadataBlockServiceBean

* Added: release notes for #10741

* Fixed: displayOnCreate query logic

* Fixed: excluding conditionally required fields when display-on-create is true

* Fixed: query predicate for required-in-dataverse field condition

* Fixed: addDataverse API facetIds field json structure

* Added: docs #10800
@GPortas GPortas added SPA.Q3 Not related to any specific Q3 feature Original size: 10 labels Sep 19, 2024
bjosel added a commit to TIK-NFL/dataverse that referenced this issue Nov 19, 2024
* missing empty watermark entry

* fix capitalization

* Changed: dataverse image_url Solr property set on SearchServiceBean

* Changed: do not modify existing JSF logic

* remove unused imports IQSS#10517

* add test to assert capitalizataion of Dataset and Software IQSS#10517

* add details to error messages (IQSS#10813)

* Fix addDataverse expected request body structure (IQSS#10802)

* Fixed: MetadataBlockServiceBean to check for not excluded fields in input levels

* Changed: using queries for obtaining dataset field types based on displaying conditions

* Refactor: json printer method for MetadataBlock

* Added: IT test case for list metadata blocks testing field with include=false and displayOnCreate=true property

* Fixed: removed condition in MetadataBlockServiceBean

* Added: release notes for IQSS#10741

* Fixed: displayOnCreate query logic

* Fixed: excluding conditionally required fields when display-on-create is true

* Fixed: query predicate for required-in-dataverse field condition

* Fixed: addDataverse API facetIds field json structure

* Added: docs IQSS#10800

* A one line fix for IQSS#10821 - ? (IQSS#10823)

* Add thumbnail for featured dataverses (IQSS#10433)

* Add thumbnail for futured dataverses

* Add documentation

* Release note snippet

* New flyway namming

* Update doc/release-notes/10433-release-notes.md

Co-authored-by: Philip Durbin <[email protected]>

* Release note snippet update
Add new recommandations (HTML preview + "for more information ...")

* Update SQL file name after 6.2 release

* renamed sql file

---------

Co-authored-by: Philip Durbin <[email protected]>
Co-authored-by: Ludovic DANIEL <[email protected]>
Co-authored-by: Philip Durbin <[email protected]>

* bump sql script version IQSS#10517

* JDD Metrics: Label KO IQSS#10123 (IQSS#10124)

* remove parentheses

* Correction of the parenthesis display

* conditional INSERT of dataset type IQSS#10517

* Add logic to suppress query tool display for non-public files.

* typo

* fix test

* fix labels when cvoc is used

* doc tweaks for versioned base images: making releases IQSS#10827

* iterate on "supported image tags" section IQSS#10827

* Added: setting imageUrl in SearchServiceBean for datasets and files

* simplify now that everything is inside the try

* update tests - added one field in citation block

* reworked controlled vocab language keys

* fixing key to lowercase

* fixing key to lowercase

* release note

* undo changes

* Update doc/release-notes/10810-search-api-payload-extensions.md

Co-authored-by: Philip Durbin <[email protected]>

* Added: note about upcoming change to image_url field in docs

* fixing language list

* fixing language list

* fixing language list

* fixing language list

* fixing language list

* fixing language list

* changes per review comments

* changes per review comments

* support no pubIdType for URLs

* direct people to the log for failures - they aren't in the response

* bug - the _target url isn't being set elsewhere

* avoid failing when the entity is null for error statuses

* don't update unpublished files - no need and it will fail

the updateIdentifier call is checking for the findable metadata which is
not available before publication.
(We don't update DataCite after dataset edits, so unpublished datasets
don't go through here, but unpublished files on published datasets would
hit this code)

* lower logging, add null check on relatedIdentifier

* Change to use POST for all

* Documentation and updated release note

* changes per review comments

* test fix - number of fields

* Remap oai_dc fields dc:type and dc:date (IQSS#10737)

* Remap oai_dc fields dc:type, dc:date, and dc:rights IQSS#8129.

The `oai_dc` export and harvesting format has had the following fields remapped:

- dc:type was mapped to the field "Kind of Data". Now it is hard-coded to the word "Dataset".
- dc:date was mapped to the field "Production Date" when available and otherwise to "Publication Date". Now it is mapped only to the field "Publication Date".
- dc:rights was not mapped to anything. Now it is mapped (when available) to terms of use, restrictions, and license.

* add tests for export and citation date IQSS#8129

* map dc:date to pub date or field for citation date  IQSS#8129

* back out of any changes to dc:rights IQSS#8129

* remove OAI-PMH changes from API changelog (also in release note) IQSS#8129

* tweak release note, mention backward incompatibility, reexport IQSS#8129

* update release note

* check for ROR in grantAgency field too

* 10527 404static.xhtml has an old date in the footer (2023 is hard-coded) and update URLs (IQSS#10535)

* Javascript updates the year automatically and the URLs are customised.

* add taps

* only change year and and all the other errors are still there.

* adopt using CDI, fix funderIdentifier element per schema

* datasetTypes test fix

* release note/changelog changes

* chore(ct): update base image wait4x to 2.14.2 for stdlib update

* chore(ct): update base image wait4x to 2.14.2 for stdlib update IQSS#10844

* don't send contributors w/o contributorType

* relatedIdentifierType is required

* flip to prefer identifier over url

seeing cases at QDR where the type is DOI, the identifier is the doi and
the URL is a non-DOI reference (e.g. at pubmed).

* Handle case where type is set but there's no identifier

* map non-standard contributors to Other, remove unused imports

* Treat missing contrib type as Other

* chore(ci): delete duplicate action after renaming

* docs(ct): update base image tag policy from latest discussion IQSS#10827

As discussed during the 2024-09-12 containerization working group meeting (see ct.gdcc.io) and on https://dataverse.zulipchat.com/#narrow/stream/375812-containers/topic/change.20version.20scheme.20base.20image.3F/near/469884104

* style,docs(ct): minor tweaks to base image policy as per @pdurbin

* feat(ct): switch latest to unstable in base image flow

As per latest discussion, we want to keep the unstable tag around.
It shall still point to the latest from develop as it has been done
before our revised tagging policy.

Latest will be used for production images, much more aligned with the
Bitnami policy

* style,docs(ct): minor tweaks to base image policy as per @pdurbin

* fix(ct): remove auth for revision action

Maybe we won't run into a rate limit for now, as the limit of 180/s * 100 = 18000 tags/s seems to leave us some headroom for now.

* fix(ct): try to avoid shell substitution in base image flow

We might need more backslashes to avoid telling the shell to replace what we want to be a maven property

* fix(ct): avoid shell substitutions going awry for base image release tag

Instead of using a Maven reference and dealing with escaping of $ chars, override the suffix with an empty string

* add trailing / after hostname for perma base-url IQSS#10775

* ci(ct): remove out of scope actions for IQSS#10478

We will deal with shipping the updated application container images separately as part of issue IQSS#10618.

Adding some comments about why some stuff is still around.

* ci(ct): reorganise tags for develop branch IQSS#10478

Now adding the "upcoming" tag during the develop branch run.

Also some reorganizing to ship the tag options using outputs, not env vars.
Moving the common Maven option to enable tag overrides to the command instead of the options building.

* chore(parent): remove OSS plugin snapshot repo after upgrade to DMP 0.45

* build(ct): make application image use new base image flexi stack IQSS#10478

* style(ct): remove stale comment from base module pom

* chore(ct): add comment about apt sec updates detection via list hack

For now, we stick to not alter the image more than necessary. Only packages we installed will be upgraded, as these are not part of the normal Java base images.

The Java base images receive regular updates and undergo testing. It might be unwise to just install all the security updates we could get.

Leaving the option here for later saves the trouble to dig up the solution again.

* ci(ct): use new setup-maven action in base image push workflow

Simplify setup following DRY principle

* ci(ct): replace logic in base image push workflow

Using the same actions and steps as done in the maintenance action should work for this workflow in case of a push event, too.

* test(ct): temporarily set dev branch in base image push flow to feature branch

* ci(ct): use setup-maven action in app image push workflow to simplify setup

Following DRY principle, reuse the steps defined

* test(ct): temporarily enable app image push flow execution in all forks

* avoid spurious log warning for others e.g. isbn

these were going through the default check for URLs and failing (not a
url) leading to a warning. The new code should try URL parsing for URLs,
try PID and URL parsing for ones with no type specified, and send the
rest of the identifiers w/o any additional (optional) attributes.

* update doc

* ci(ct): use an optional base image ref for app image push flow

We need to transfer the determined base image name we might have just built from the calling workflow into this flow.
As we provide a default value, this is picked up for pull_requests.

* ci(setup-maven): try to auto-detect git ref

It's not so easy to determine the right git ref for different scenarios like PR, etc. Unless explicitly given a ref, try to autodetect the right one to go with.

* chore(ct): add notes in flows about adding a path filter

We want to avoid duplicate runs which might trigger race conditions for image shipments.

* chore(ct): add note about missing triggers for base push flow

This is out of scope for IQSS#10827, but should be addressed at a later point to avoid duplicated runs with potential race conditions. Also it enables proper rebuilds for preview images when someone is just trying to create a base image change, but which should obviously be tested full chain.

* fix,ci(ct): don't trigger the base push flow for backports

Obviously these are meant for the maintenance workflow, not the push flow!

* ci(ct): trigger app flow from base push flow

We detect the tag we have been using in a finalizing step to hand a proper base image ref to the app image workflow to make it work on the images we just pushed to the registry.

* changes per review

* Apply suggestions from code review

Co-authored-by: Philip Durbin <[email protected]>

* cleaner formatting

* minor doc tweak IQSS#10632

* standardize image url

* No longer needed with use of CDI.current() in XMLMetadataTemplate

* no longer used and CrossRef ended up using it's own.

* add more info about the scope of changes.

* doc changes

* doc changes

* ci(ct): reshape maintenance workflow into external matrix script

Unfortunately, matrix jobs logs and outputs cannot be aggregated in Github Actions.

The only way to work around the limitations of GHA is by using a custom build script that create a similar matrix like experience.

This commit introduces these scripts, probably also making some custom actions we added obsolete.

* ci(ct): remove obsolete actions for revisions and parent image changes detection

* ci(ct): re-enable forced build for maintenance workflow

* doc changes

* doc changes

* ci(ct): add outputs to maintenance matrix job

Can be picked up by other jobs, e.g. to create textblocks for docs or a job matrix.

* ci(ct): remove draft of building app images in maintenance matrix job

* style,ci(ct): reword the maintenance build workflow name

* ci(ct): make the maintenance workflow push the hub description for the base image IQSS#10478

* style(ct): fix simple typo in base image README

* fix(ct): remove bug from package upgrade detection in maintenance workflow

We did not correctly compare the status code of the grep command, breaking the update detection

* update query per review comments to handle all cases

* docs,style(ct): small rewording about immutable tags for base image

* feat,ci(ct): add immutable tags to list of base image tags in maintenance job

As discussed during community meeting on 2024-09-19.

* style,ci(ct): add some more verbosity about progress in maintenance job

* refactor,ci(ct): finishing touches for IQSS#10478

Re-enable and change everything necessary to reference the upstream IQSS context as of now.

* add docs for disable-dataset-thumbnail-autoselect IQSS#10819 IQSS#10820

* create 6.4 release notes and add about half the updates IQSS#10853

* add second half of snippets IQSS#10853

* fix,ci(ct): only add base image Maven option when the input is defined

Without this in case of the pull_request event the input is null and the build fails because we have base image defined at all.

Simply not adding the option if the input is undefined means we stick to what is defined within the POM.

* add highlights and upgrade steps IQSS#10853

* docs(ct): add release note for maintenance workflow IQSS#10478

* style(ct): add comment explaining what a flavor is in base image Maven props

Co-authored-by: Philip Durbin <[email protected]>

* A potential simple fix for IQSS#10667 ?

* Update 6.4-release-notes.md

corrected the schema.xml instructions

* add blurb for tagged base images IQSS#10853

* doc how to handle develop into develop PRs IQSS#9508

* remove extra line

* dont return image_url if there is none

* Fix NPE using CVOC

* set dataset type before registering pid (which needs the type)

* add release note

* adding fix from review comment

* remove tabs to make reviewdog happy. woof! IQSS#10623

* globus doc tweaks IQSS#10623

* add more and better DataCite export IQSS#10853

* add blur for listing feature flags IQSS#10853

* add CVOC bug fix IQSS#10853
-6.4-release-notes

* update image_url IQSS#10853

* Updated the docs to reflect the new name of a JVM option (IQSS#10623)

* I fixed anchor links IQSS#10876 (IQSS#10877)

* improve release note IQSS#10623

* add globus async IQSS#10853

* add cvoc update IQSS#10853

* add new globus settings under settings section IQSS#10853

* various tweaks IQSS#10853

* typo IQSS#10853

* put features before bug fixes IQSS#10853

* reword

* datacite title

* croissant update IQSS#10853

* bump version to 6.4 IQSS#10852 (IQSS#10871)

* displayOnCreate set to true for depositor and dateOfDeposit in Citation metadata block (IQSS#10884)

* Changed: displayOnCreate set to true for depositor and dateOfDeposit in citation.tsv

* Changed: MetadataBlocksIT test assertion for new total number of displayOnCreate fields

* Added: release notes for IQSS#10850

* Added: minor tweak to release notes

* IQSS#10853 fix typo version number

* Add release note change for fields depositor and dateOfDeposit in the citation.tsv

* remove old release note

* formatting fix

fixed formatting of the shell block in the upgrade instruction

* tweak depositor and dateOfDeposit IQSS#10853

* fixed update-fields.sh url 

(it had "9.4.1" in it; and we probably don't want to get it from the master branch either)

* reindex instruction

* removed a superfluous command line

* temp dir cleanup

* typo

* docs: update release notes from IQSS#10343

* tweaks IQSS#10343

* Upgrade to upstream version 6.4

* Merge upstream v6.4 into branch properties

* Sync with upstream

* Fix merge of properties

* bugfix: metadataFragment.xhtml

* fix relationType display value bug

---------

Co-authored-by: Jim Myers <[email protected]>
Co-authored-by: GPortas <[email protected]>
Co-authored-by: Philip Durbin <[email protected]>
Co-authored-by: landreev <[email protected]>
Co-authored-by: jeromeroucou <[email protected]>
Co-authored-by: Philip Durbin <[email protected]>
Co-authored-by: Ludovic DANIEL <[email protected]>
Co-authored-by: sbondka <[email protected]>
Co-authored-by: Stephen Kraffmiller <[email protected]>
Co-authored-by: Steven Winship <[email protected]>
Co-authored-by: Benedikt Kruse <[email protected]>
Co-authored-by: Oliver Bertuch <[email protected]>
Co-authored-by: Oliver Bertuch <[email protected]>
Co-authored-by: qqmyers <[email protected]>
Co-authored-by: paulboon <[email protected]>
Co-authored-by: ofahimIQSS <[email protected]>
Co-authored-by: Florian Fritze <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GREI Re-arch Issues related to the GREI Dataverse rearchitecture Original size: 10 Size: 10 A percentage of a sprint. 7 hours. SPA.Q3 Not related to any specific Q3 feature SPA These changes are required for the Dataverse SPA Type: Bug a defect
Projects
Status: No status
2 participants