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

Edit Metadata API endpoint - not allowing empty values #11243

Open
g-saracca opened this issue Feb 11, 2025 · 8 comments · May be fixed by #11273
Open

Edit Metadata API endpoint - not allowing empty values #11243

g-saracca opened this issue Feb 11, 2025 · 8 comments · May be fixed by #11273
Assignees
Labels
FY25 Sprint 17 FY25 Sprint 17 (2025-02-12 - 2025-02-26) GREI Re-arch Issues related to the GREI Dataverse rearchitecture Original size: 30 Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) SPA.Q1 Not related to any specific Q1 feature SPA These changes are required for the Dataverse SPA Type: Bug a defect

Comments

@g-saracca
Copy link
Contributor

g-saracca commented Feb 11, 2025

What steps does it take to reproduce the issue?
Try to update a dataset metadata sending a payload with empty values like:

[
...rest of required fields,
{
    "value": "",
    "typeClass": "primitive",
    "multiple": false,
    "typeName": "subtitle"
}]

And you will receive an error like:

{
    "status": "ERROR",
    "message": "Error parsing dataset update: Empty value for field: Subtitle"
}
  • What happens?
    API endpoint is returning an error, but on updates it should allow empty values on not required fields as users may want to remove a value from a field and leave it empty.

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

  • What did you expect to happen?
    Not to receive an error and update the metadata correctly.

Which version of Dataverse are you using?
unstable

Any related open or closed issues to this bug report?
After this gets fixed we should continue with frontend issue IQSS/dataverse-frontend#598.

Screenshots:

Image

Image

Image

@g-saracca g-saracca added GREI Re-arch Issues related to the GREI Dataverse rearchitecture Original size: 3 Size: 3 A percentage of a sprint. 2.1 hours. SPA These changes are required for the Dataverse SPA SPA.Q1 Not related to any specific Q1 feature Type: Bug a defect labels Feb 11, 2025
@GPortas GPortas moved this to SPRINT READY in IQSS Dataverse Project Feb 12, 2025
@g-saracca g-saracca moved this from SPRINT READY to This Sprint 🏃‍♀️ 🏃 in IQSS Dataverse Project Feb 12, 2025
@cmbz cmbz added the FY25 Sprint 17 FY25 Sprint 17 (2025-02-12 - 2025-02-26) label Feb 12, 2025
@pdurbin
Copy link
Member

pdurbin commented Feb 14, 2025

I poked around with this a bit.

It's possible to delete a subtitle, for example, using this API: https://guides.dataverse.org/en/6.5/api/native-api.html#delete-dataset-metadata

As the docs, say, it has to be an exact match. This is the JSON I used:

{
  "fields": [
    {
      "typeName": "subtitle",
      "value": "mySubtitle"
    }
  ]
}

This and related APIs were added in this PR:

@g-saracca
Copy link
Contributor Author

Hi @pdurbin , thanks for those recommendations, but we need to stick to the edit metadata endpoint we were using, as it works with a similar DTO as the create dataset endpoint and we are also reusing the form component in the SPA for both creating and editing a dataset.

@GPortas GPortas self-assigned this Feb 17, 2025
@GPortas GPortas moved this from This Sprint 🏃‍♀️ 🏃 to In Progress 💻 in IQSS Dataverse Project Feb 17, 2025
@GPortas GPortas added Size: 10 A percentage of a sprint. 7 hours. Original size: 10 and removed Size: 3 A percentage of a sprint. 2.1 hours. Original size: 3 labels Feb 18, 2025
@GPortas GPortas linked a pull request Feb 19, 2025 that will close this issue
@qqmyers
Copy link
Member

qqmyers commented Feb 19, 2025

How will this work with multiple values? Do you have to send all of the existing ones to know which is now blank?

@GPortas
Copy link
Contributor

GPortas commented Feb 19, 2025

How will this work with multiple values? Do you have to send all of the existing ones to know which is now blank?

Currently, it's the preferred way I see to do it. Do you see any downside apart from having to know and send the values you want to keep?

@qqmyers
Copy link
Member

qqmyers commented Feb 19, 2025

I guess if one more is added since you last looked, the delete becomes ambiguous. As for alternatives, as Phil said we do have the delete api call, but if it helps to allow this in edit, I could imagine a delete flag, e.g. sending the original value that you're removing with a delete=true versus sending an empty value. (I don't recall - did we ever decide on how to be sure you have the latest info when making api calls, which is what keeping the datasetversion.version integer does for the current UI (causing OpportunisticLockException if your version is behind the one on the db due to someone else's change.)).

@GPortas GPortas added Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) Original size: 30 and removed Size: 10 A percentage of a sprint. 7 hours. Original size: 10 labels Feb 20, 2025
@GPortas
Copy link
Contributor

GPortas commented Feb 20, 2025

@qqmyers I've been discussing with @g-saracca, and we believe the best approach is to send the API the datasetversion.version integer along with the updated metadata fields. The API would then verify that no consecutive version numbers have been registered, returning an error if they have.

This way, the payload would include only empty values for deletions, eliminating concerns about unintentionally deleting unknown data, as the user must have the latest version for the update to proceed. I have resized the issue to incorporate this check within its scope.

@qqmyers
Copy link
Member

qqmyers commented Feb 20, 2025

Is this something we want across the API, e.g. version as an optional (for backward compatibility) param on anything that updates the datasetversion? Not to slow this issue down, but maybe put the check in the Abstract api bean so it can be reused in other calls?

@GPortas
Copy link
Contributor

GPortas commented Feb 21, 2025

Yes, definitely. At least design the solution to be reusable from other endpoints within the scope of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FY25 Sprint 17 FY25 Sprint 17 (2025-02-12 - 2025-02-26) GREI Re-arch Issues related to the GREI Dataverse rearchitecture Original size: 30 Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) SPA.Q1 Not related to any specific Q1 feature SPA These changes are required for the Dataverse SPA Type: Bug a defect
Projects
Status: In Progress 💻
Development

Successfully merging a pull request may close this issue.

5 participants