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

Fixed field name editing #566

Merged
merged 1 commit into from
Sep 30, 2024
Merged

Fixed field name editing #566

merged 1 commit into from
Sep 30, 2024

Conversation

roll
Copy link
Collaborator

@roll roll commented Sep 25, 2024


Please make sure that all the checks pass. Please add here any additional information regarding this pull request. It's highly recommended that you link this PR to an issue (please create one if it doesn't exist for this PR)

@roll roll requested a review from guergana September 25, 2024 11:01
Copy link

Deploying opendataeditor with  Cloudflare Pages  Cloudflare Pages

Latest commit: 55a1a80
Status: ✅  Deploy successful!
Preview URL: https://174a857a.opendataeditor.pages.dev
Branch Preview URL: https://562-fix-editing-column-names.opendataeditor.pages.dev

View logs

Copy link
Collaborator

@guergana guergana left a comment

Choose a reason for hiding this comment

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

Great job, @roll , it works now but shouldn't this change include also updating the name of the column to reflect the empty column name? with this fix now an error shows but the column name looks the same in the tabular representation, so it's kinda hard to see what is the error:

image

@roll
Copy link
Collaborator Author

roll commented Sep 27, 2024

Thanks @guergana!

It's there it just needs better UI and error message:

Screenshot from 2024-09-27 10-11-21

PS.
Also, created this issue - #571

@romicolman
Copy link
Collaborator

Hi @roll and @guergana! I tested this PR.

The implementation works. In this sense, the ODE does not crash when changing the column name. However, if I go to the metadata panel, Schema--Fields and change the name of the column, the info is not updated. Should I create a separate issue for this @roll so we can merge this PR?

Video showing the problem:

Grabacion.de.pantalla.2024-09-27.a.la.s.2.53.11.p.m.mov

@roll
Copy link
Collaborator Author

roll commented Sep 30, 2024

@romicolman
Thanks!

Yes, let's create a separate issue because current behaviour is correct regarding the initial concept of how data and metadata editing co-exist. In this case:

  • you edit metadata but not data so the field name now is "new_name"
  • but data is not changed so it show a validation error

Probably we need to change it concept. Also I created this issue - #571

Copy link
Collaborator

@guergana guergana left a comment

Choose a reason for hiding this comment

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

Thanks! ✔️

Copy link
Collaborator

@romicolman romicolman left a comment

Choose a reason for hiding this comment

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

New issue created :) #574

@roll
Copy link
Collaborator Author

roll commented Sep 30, 2024

Thanks!

@roll roll merged commit f9b2370 into main Sep 30, 2024
9 checks passed
@roll roll deleted the 562/fix-editing-column-names branch September 30, 2024 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ODE stops working when editing column names
3 participants