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

added protectionsState to breakage form submissions #1790

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

shakyShane
Copy link
Collaborator

@shakyShane shakyShane commented Oct 31, 2023

Task/Issue URL: https://app.asana.com/0/0/1205839123798770/f
Tech Design URL:
CC:

Description:

Adds a new parameter to breakage form submissions

  • added the param when the form is submitted from the privacy dashboard
  • added the param when the form is submitted from the native feedback form

Steps to test this PR:

  1. submit the form from either place
  2. verify that the param protectionsState has either a "0" or "1" value based on the rules mentioned in the Asana task.

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

@shakyShane
Copy link
Collaborator Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@shakyShane shakyShane requested review from ayoy and miasma13 and removed request for ayoy November 1, 2023 13:33
Comment on lines +40 to +43
enum ProtectionsState: String {
case enabled = "1"
case disabled = "0"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Not sure what is the expected way of extending the values range here, but maybe a simple bool would suffice?
  2. Either with the above change or not I suggest replacing "1"/"0" to "true"/"false" to be consistent with other parameters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@miasma13 as we discussed in MM - this mapping to "1" | "0" was done intentionally to match what 'other' platforms are doing with this parameter - since I'm adding to all 5 :)

@shakyShane shakyShane merged commit da25db0 into develop Nov 7, 2023
15 checks passed
@shakyShane shakyShane deleted the shane/protection-state branch November 7, 2023 09:36
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