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 saving errored tables #648

Merged
merged 29 commits into from
Nov 26, 2024
Merged

Fix saving errored tables #648

merged 29 commits into from
Nov 26, 2024

Conversation

roll
Copy link
Collaborator

@roll roll commented Nov 18, 2024


It's quite a fragile solution that might required to be extended in the future for other types of errors. On the other hand, it seems to be the only solution that's possible with current data flow.

This PR also adds a proper loading/error state management for the saving changes action as after the change this action might be really time-consuming for large files.

@roll roll requested a review from guergana November 18, 2024 14:27
@roll roll requested a review from romicolman November 18, 2024 14:27
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.

Hi @roll! Just to see if what I am seeing is correct. I ingested the same file, went to row 128, edited the content and then clicked Save changes. The ODE changed the content of the cell and removed the content from the rest of the cells that had the same data. Is this correct?

Grabacion.de.pantalla.2024-11-21.a.la.s.9.15.34.a.m.mov

@roll
Copy link
Collaborator Author

roll commented Nov 25, 2024

Hi @romicolman,

can you please share the file. Is it the same? I can't reproduce the problem:

Untitled.Video.November.25.2024.1_49.PM.webm

@romicolman
Copy link
Collaborator

Yes, here is the file:

Distribucio.de.la.renda.P80_P20 (2).xlsx

@roll
Copy link
Collaborator Author

roll commented Nov 26, 2024

Still can't reproduce it... I do the same actions and it goes from 89 errors to 88 errors as expected.

@guergana can you please try (uploading this file, fixing one of errors and saving it)?

@roll
Copy link
Collaborator Author

roll commented Nov 26, 2024

@guergana
Nevermind, we solved the problem

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.

Approved

@roll
Copy link
Collaborator Author

roll commented Nov 26, 2024

Thanks!

@roll roll merged commit c08961b into main Nov 26, 2024
9 checks passed
@roll roll deleted the 614/fix-saving-errored-tables branch November 26, 2024 13:26
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.

Unexpected behaviour with Save Changes action
2 participants