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

Send null for string-like columns that don't allow empty strings when cell is cleared #3784

Open
pavish opened this issue Aug 26, 2024 · 6 comments · May be fixed by #4030
Open

Send null for string-like columns that don't allow empty strings when cell is cleared #3784

pavish opened this issue Aug 26, 2024 · 6 comments · May be fixed by #4030
Labels
help wanted Community contributors can implement this ready Ready for implementation type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory
Milestone

Comments

@pavish
Copy link
Member

pavish commented Aug 26, 2024

Description

  • Open or create a table with a column that allows string input that performs some validation.
    • Date & Time
    • Date
    • Email
    • URI etc.,
  • Let's say we have a URI column.
  • Edit a cell and enter some string., eg., "test".
  • This will result in an error. That's expected.
  • Now edit the cell again and remove everything clearing the cell.
  • This will result in an error again, when it shouldn't.
  • That's because the frontend sends the patch request with an empty string "".
  • (The user can still right click and set to null, but that's not always obvious in this situation).
  • For such columns that have custom validation and do not allow empty strings, the frontend should send null instead of empty string "".
@pavish pavish added type: bug Something isn't working help wanted Community contributors can implement this work: frontend Related to frontend code in the mathesar_ui directory labels Aug 26, 2024
@seancolsen seancolsen added the ready Ready for implementation label Aug 28, 2024
@seancolsen seancolsen added this to the Backlog milestone Aug 28, 2024
@cmagapu
Copy link

cmagapu commented Sep 15, 2024

Hi @pavish @seancolsen I'm new to open source and I would like to contribute to this.

@seancolsen
Copy link
Contributor

Sure, @cmagapu you're welcome to submit a PR for this. Contact us on Matrix if you have any questions.

I'll say though that I don't expect this issue to be super straightforward or easy for new contributors. The tricky part is that for some types we need to send "" while for other types we need to send null. Finding a clean place encode that delineation might require some nuanced tradeoffs and deep understanding of a particularly messy area of our codebase.

@aniketkulkarni17
Copy link
Contributor

Hi,
If this issue still needs to be worked on, I would like to give it a try. Is it possible to assign it to me?

@seancolsen
Copy link
Contributor

@aniketkulkarni17 We don't assign issues to new contributors, but you can definitely still work on this. Feel free to submit a PR.

@aniketkulkarni17
Copy link
Contributor

Hi @seancolsen ,
I'm working on this issue and have potentially fixed it. When the user enters a "" value, the frontend will send null instead of "" and no error is raised. However, before raising the PR, I would like to know which datatypes should this logic be applicable to?

@seancolsen
Copy link
Contributor

Good question @aniketkulkarni17. I just played around with all the types. It seems that this problem is present for the following types:

  • Date
  • Date and time
  • Time
  • Email
  • URI

Interestingly, the Duration type actually already works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Community contributors can implement this ready Ready for implementation type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory
Projects
None yet
4 participants