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

Pre-validate hooks not running #1814

Closed
mantariksh opened this issue May 6, 2021 · 2 comments · Fixed by #2584
Closed

Pre-validate hooks not running #1814

mantariksh opened this issue May 6, 2021 · 2 comments · Fixed by #2584
Assignees
Labels
bug Something isn't working contribute free for contributors to pick up P2 planned for next 1-2 months

Comments

@mantariksh
Copy link
Contributor

Due to the new usage of findByIdAndUpdate, certain pre-validate and pre-save hooks are not running as findByIdAndUpdate does not activate them. For example, the Form model has a check to ensure that the total form size does not exceed 10MB, but this check does not run when the Settings page is updated as the settings update uses findByIdAndUpdate. The Verification model also has a check for duplicate field IDs which is not run when Verification documents are updated.

The following fixes should be made to ensure that the functionality in the hooks is maintained:

  1. For the Form model, the check for object size should be moved in to the Dropdown and Checkbox fields, which have a risk of becoming too large as the user is allowed to copy and paste multiple options into a textarea.
  2. For the Form model, the checks for the admin and permissionList should be moved into the admin and permissionList keys.
  3. For the Verification model, the check for duplicate field IDs should be moved into the fields key.
  4. All other instances of findByIdAndUpdate and findOneAndUpdate should be checked to ensure that the models do not rely on Mongoose hooks to maintain the correct state.
@r00dgirl r00dgirl added contribute free for contributors to pick up bug Something isn't working labels May 11, 2021
@frankchn frankchn self-assigned this May 11, 2021
@r00dgirl r00dgirl added the P2 planned for next 1-2 months label May 11, 2021
@frankchn
Copy link
Contributor

For (1), the user could also exceed the object size in other ways right (by just having lots of helper text, etc...)? Is there a more robust way of doing this?

@liangyuanruo
Copy link
Contributor

Discussion

  1. Use a heuristic e.g. max length to prevent users from uploading in the UI by disabling the "Save" button
  2. Remove complicated bson size estimation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working contribute free for contributors to pick up P2 planned for next 1-2 months
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants