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

Enhance offliners definitions #834

Merged
merged 5 commits into from
Sep 29, 2023
Merged

Enhance offliners definitions #834

merged 5 commits into from
Sep 29, 2023

Conversation

benoit74
Copy link
Collaborator

@benoit74 benoit74 commented Sep 28, 2023

Rationale

Changes

  • all description and long_description fields are now checked with the appropriate validator
  • long_description are now a LongString field, so that the UI is populated properly with a text area
  • FCC language field is now validated with a oneof validator, which will be reflected in the /offliners API as a choices property and in the UI as a combobox instead of a text field
  • All marshmallow fields.String are replaced by a custom String which forbids the presence of Unicode null character in the value at validation ; this allows to fix issue for all API endpoints, even if some values would have been rejected by postgreSQL (unicode null string is not allowed in VARCAHR columns, but it is in JSON columns) it allows for much nicer error messages

@benoit74 benoit74 force-pushed the enhance_offliners_defs branch 2 times, most recently from b98e374 to 25ed6d9 Compare September 28, 2023 13:31
@benoit74 benoit74 marked this pull request as ready for review September 28, 2023 13:42
@benoit74 benoit74 force-pushed the enhance_offliners_defs branch from 25ed6d9 to 35a4d4c Compare September 28, 2023 15:59
@benoit74 benoit74 requested a review from rgaudin September 28, 2023 15:59
Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

I think you missed one validator

@benoit74 benoit74 force-pushed the enhance_offliners_defs branch from 7ba211a to ad6c5a4 Compare September 28, 2023 18:48
@benoit74 benoit74 requested a review from rgaudin September 29, 2023 08:39
Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

👍

@rgaudin rgaudin merged commit 576aca3 into main Sep 29, 2023
5 checks passed
@rgaudin rgaudin deleted the enhance_offliners_defs branch September 29, 2023 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants