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

feat: Add a limit for the number of MyInfo fields that can be added #1664

Merged
merged 7 commits into from
Apr 26, 2021

Conversation

frankchn
Copy link
Contributor

@frankchn frankchn commented Apr 17, 2021

Problem

MyInfo fields must be limited to 30 per form on Email mode to comply with PII guidelines. In exchange, we get access to all fields.

Closes #1305

Solution

Adds limits and validators to the appropriate places to limit MyInfo fields to 30.

Screenshots

image

image

Tests

  • Add 30 MyInfo fields to a form.
  • Check that under the build tab, MyInfo fields can no longer be added
  • Check that MyInfo fields cannot be duplicated
  • Delete a Myinfo field (such that the form now has <30) and check that you can now add and duplicate Myinfo fields
  • Manually add an additional MyInfo field to the form by editing the database. Check that when you try to edit the form subsequently, you get an error message saying to check that the form has max 30 MyInfo fields.
  • Run the following query to ensure that there are no existing forms with >30 MyInfo fields:
db.getMongo().setReadPref('secondary')
db.getCollection('forms').aggregate([
    { $match: { 'form_fields.myInfo': { $exists: true } } },
    { $unwind: '$form_fields' },
    { $match: { 'form_fields.myInfo': { $exists: true } } },
    { $group: { _id: '$_id', count: { $sum: 1 } } },
    { $match: { count: { $gt: 30 } } }
])
  • After release has been deployed, run the above query again to check that there are still no forms with >30 MyInfo fields. If there are, the form admin must be contacted.

….view.html

Co-authored-by: Antariksh Mahajan <antarikshmahajan@gmail.com>
Copy link
Contributor

@mantariksh mantariksh left a comment

Choose a reason for hiding this comment

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

thanks @frankchn! this looks good, but I'll need to write and run a database query first to check that there aren't already forms with >30 MyInfo fields. if the current database state checks out then I'll approve and merge this. pending last comment below

@liangyuanruo
Copy link
Contributor

thanks @frankchn! this looks good, but I'll need to write and run a database query first to check that there aren't already forms with >30 MyInfo fields. if the current database state checks out then I'll approve and merge this.

Update on behalf on @mantariksh - there aren't any such forms, so the PR is safe.

src/app/models/form.server.model.ts Outdated Show resolved Hide resolved
src/app/models/form.server.model.ts Outdated Show resolved Hide resolved
frankchn and others added 2 commits April 21, 2021 22:32
Co-authored-by: Antariksh Mahajan <antarikshmahajan@gmail.com>
Copy link
Contributor

@mantariksh mantariksh left a comment

Choose a reason for hiding this comment

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

one remaining bug, otherwise looks good!

….view.html

Co-authored-by: Antariksh Mahajan <antarikshmahajan@gmail.com>
@mantariksh mantariksh merged commit 8cfb75d into develop Apr 26, 2021
@liangyuanruo liangyuanruo deleted the frank-myinfo-30 branch April 27, 2021 02:09
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.

Limit to 30 MyInfo fields
3 participants