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

Sanitize custom_stages and stages_enabled #6770

Merged
merged 11 commits into from
Feb 16, 2024
Merged

Conversation

timolegros
Copy link
Collaborator

@timolegros timolegros commented Feb 15, 2024

Link to Issue

Closes: #6718

Description of Changes

  • Updated Community.stages_enabled column type from string to boolean
    • This includes converting all undefined and empty string values to false in the DB
  • Updated Community.custom_stages column type from string to array of strings
    • This includes converting all improperly formatted stringified arrays into proper arrays in the DB
  • Updated the API to only accept and return an array for custom_stages

Test Plan

Requires FLAG_NEW_ADMIN_ONBOARDING=true to be set. Also requires a fresh database dump (yarn dump-db).

  • Run yarn db-all
  • Enable/Disable stages
  • Create stages like ['test 123', 'test 321'] and ["test one", "test two"]
  • Remove custom stages after creation

Other Considerations

The migration includes queries that convert custom_stages to an array of strings for specific communities. This is because there is no easy/clean generalized approach for converting these values. It is important to note that should these values change for the community before this migration is run, the migration may fail. That said, the client currently in production already includes validation for 99% of cases. There are 2 ways an incorrect value could be inserted:

  1. A user accesses the API directly and provides a faulty value
  2. A user defines a list on the client with single quotes instead of double quotes (client-side validation does not cover this case until this PR is merged).

@timolegros timolegros marked this pull request as ready for review February 15, 2024 21:52
Copy link
Contributor

@mzparacha mzparacha left a comment

Choose a reason for hiding this comment

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

LGTM

@lzach83
Copy link
Contributor

lzach83 commented Feb 16, 2024

LGTM

Copy link
Contributor

@CowMuon CowMuon left a comment

Choose a reason for hiding this comment

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

Approved for Common release v. 1.0 (nee v0.9.6)

@jnaviask jnaviask merged commit 18bd067 into master Feb 16, 2024
6 of 7 checks passed
@jnaviask jnaviask deleted the tim/custom-stages-validation branch February 16, 2024 21:07
jnaviask pushed a commit that referenced this pull request Feb 16, 2024
* stages_enabled column fixes

* custom_stages fixes

* validate custom stages array in API

* fix client/API custom stages

* type fixes

* fix migration

* disallow single quotes in validation

* remove quote replacement
@kurtisassad kurtisassad mentioned this pull request Feb 22, 2024
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.

Sanitize Custom Stages input and clean up database
5 participants