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: Allow screen IDs to have any format #305

Conversation

jzimbel-mbta
Copy link
Member

Asana task: Permanent Config GL E-Ink QA (Too-strict screen ID validations)

The validation code enforced a particular format on the GL e-ink screen IDs, but we don't need that.


Notes for posterity:

  • The other validation issue found during eng QA, where duplicate screen IDs were not always caught, was actually just one more symptom of the auth issue that Christian fixed in fix: User Session TTL #277.
  • I originally went down a rabbit hole trying to fix a large number of issues with state value mutation on the client, but ended up dropping that work in favor of a more surgical fix. In case we do end up finding problems caused by the mutations in the future, I've pushed my WIP, semi-broken code to the jz/fix-pending-screen-validation as a possible reference or starting point for a more complete fix.

Copy link
Contributor

@cmaddox5 cmaddox5 left a comment

Choose a reason for hiding this comment

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

May be a design/product question, but if we aren't doing these checks anymore should we bother pre-populating the field? When a new row is added, it will have EIG- in the box. Is that necessary?

@cmaddox5 cmaddox5 assigned jzimbel-mbta and unassigned cmaddox5 Apr 19, 2024
@jzimbel-mbta
Copy link
Member Author

@cmaddox5 I think it's good as-is. That's a convenience thing since in most cases the IDs will follow that format. We just don't want to stop people from using a different format if they feel the need.

@cmaddox5 cmaddox5 assigned jzimbel-mbta and unassigned cmaddox5 Apr 19, 2024
@jzimbel-mbta jzimbel-mbta merged commit f22a272 into permanent-configuration Apr 19, 2024
4 checks passed
@jzimbel-mbta jzimbel-mbta deleted the jz/fix-pending-screen-validation--minimal branch April 19, 2024 18: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
Development

Successfully merging this pull request may close these issues.

2 participants