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: Cannot update value of attribute with reserved name when it starts empty #841

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

waltton
Copy link

@waltton waltton commented Oct 22, 2024

Hi all, first PR here 👋 .

I observed this bug on a case that I have a field with the name data and I was able to create a record and update it without any issues if the first time I create the record with a value that is not empty.
If the record is created with empty value in the data field I cannot update it via admin.

Took me a while to understand the checks so I added a comment.

At the end, my solution was just removing the check and getattr(obj, reserved_field_name, None).
I thought about adding an or condition and check if the attribute had a value on the obj or form_data but I didn't saw the point of it. Maybe I'm missing something.

@aminalaee
Copy link
Owner

Thanks for the PR.
I think it is strange this is happening since we already added some logic to rename WTForm private fields internally with WTFORMS_ATTRS_REVERSED. I think a good step would be to look why that logic is not working as it was the purpose.

@waltton
Copy link
Author

waltton commented Oct 27, 2024

@aminalaee I reverted the fix so the case I'm hitting is clear on the tests I added.
Please take a look at https://github.com/aminalaee/sqladmin/pull/841/checks#step:8:105

This is the issue I'm trying to fix.

If the object being passed has a empty value, the field is not denormalized.

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.

2 participants