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

Deprecate omeroweb.custom_forms.NonASCIIForm #473

Merged
merged 1 commit into from
May 31, 2023

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented May 22, 2023

Fixes #468

The original intent of the NonASCIIForm was to support Unicode strings by overriding the CharField validation. Unicode is now natively supported by the Python/Django stack. This commit proposes to drop our intermediate custom class in favor of upstream django.forms.Form. This should incidentally reduce the maintenance burden when upgrading between Django major versions.
Testing:

All places using the legacy base form should be reviewed with this change using both ASCII strings and Unicode strings as inputs. Below is a list of endpoints to review

  • webadmin:
    • /webclient/login/
    • /webadmin/forgottenpassword/
    • /webadmin/myaccount/
    • /webadmin/experimenter/new
    • /webadmin/experimenter/edit/{id} (including Change User's Password)
    • `/webadmin/group/new
    • /webadmin/group/edit/{id}
    • /webclient/search/
  • webclient:
    • container edition (name, description)
    • object annotations (tags, key/value, comment,

The original intent of the NonASCIIForm was to support Unicode
strings by overriding the CharField validation. Unicode is now
natively supported by the Python/Django stack. This commit proposes
to drop our intermediate custom class in favor of upstream
django.forms.Form. This should incidentally reduce the maintenance burden
when upgrading between Django major versions.
Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

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

Functionally tested all the forms above on merge-ci. All working fine.

Code changes look good. 👍

@knabar knabar added this to the 5.21.0 milestone May 31, 2023
@knabar knabar merged commit ac5e4cc into ome:master May 31, 2023
8 checks passed
@sbesson sbesson deleted the nonasciiform_cleanup branch June 22, 2023 21:08
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.

Review and possible deprecate NonASCIIForm
3 participants