-
Notifications
You must be signed in to change notification settings - Fork 6
Implement edit metadata definition #977
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
Conversation
|
Something that I tried. I created a metadata definition where the input type was a text string. Then I changed it to a date picker. When I go to the file with the metadata, and I edit it, I get the new select. But if I click on the 'add metadata' button instead of editing the existing metadata, I get a blank screen. This happens both before I edit the metadata value (when it's still a string even though the definition is a DateTimePicker) or after. |
|
I also noticed that the 'Add Metadata' button is missing on the Dataset page on this branch, but it is present in main. So this branch might just need a merge with main to fix that. |
merged main |
looks like "Add metadata" button is not working on main. I created a separate issue #1033 |
longshuicy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Could you remove all the
console.log? - If the metadata definition is modified, but there are already metadata created using this definition, there might be some inconsistency. We might want to think about what should be the behavior
frontend/src/components/metadata/EditMetadataDefinitionModal.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/metadata/EditMetadataDefinitionModal.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/metadata/EditMetadataDefinitionModal.tsx
Outdated
Show resolved
Hide resolved
|
This Pr needs some more work related to editing metadata defn if already some metadata exists with old defn. It’s listed here in the issue #934. I will address the comments together. Marking it draft for now. |
longshuicy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works well. Thanks!
|
works good, doesnt let me edit as wanted if a metadata already exists 😄 |

Steps to test: