Repository navigation
Dendencies: Update DOMPurify to 3.4.17 - #8731
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Update DOMPurify dependency range specifyweb/frontend/js_src/package.json |
The manifest changes the DOMPurify dependency range from ^3.4.14 to ^3.4.17. |
Suggested reviewers: g1rly-c0d3r, jdam2k4, kwhuber
Priority:
Change: Other
Merge Risk: 
0f021
-
The dependency update has no identified merge-blocking issue and is mergeable subject to normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Testing Instructions The PR changes the frontend dependency and lockfile from DOMPurify 3.4.14 to 3.4.17, but the PR description provides no testing steps. The affected component is specifyweb/frontend/js_src; its packa…Add clear testing instructions. State that commands run from specifyweb/frontend/js_src, install the locked dependencies withnpm ci, runnpm test, and perform a frontend smoke test of the DOM-sanitization paths that use DOMPurify. Re…-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the change: updating the DOMPurify dependency to 3.4.17. The raw summary confirms that the only changed file, package.json, moves the dompurify range from ^3.4.14 to ^3.4.17. The title… Linked Issues check Issue #8730directly requests an update to DOMPurify. The PR changesspecifyweb/frontend/js_src/package.jsonfrom^3.4.14to^3.4.17. Issue#8730provides no additional bug details, acceptance c…Out of Scope Changes check The reviewed change is limited to the dompurifydependency declaration inspecifyweb/frontend/js_src/package.json. This change directly supports issue#8730. No unrelated change is shown. `package…Automatic Tests PASS. The PR only updates the DOMPurify version in package.json and package-lock.json. It does not change application logic or introduce a new behavior that requires an automatic test. The frontend al… Full details: Title check
-
Explanation
The title names the change: updating the DOMPurify dependency to 3.4.17. The raw summary confirms that the only changed file, package.json, moves the dompurify range from ^3.4.14 to ^3.4.17. The title matches the main change. It contains a typo: "Dendencies" should read "Dependencies". The PR objectives also use the misspelled form, so the typo is in the title itself. The typo does not change the meaning, so the check passes.
Full details: Linked Issues check
-
Explanation
Issue
#8730directly requests an update to DOMPurify. The PR changesspecifyweb/frontend/js_src/package.jsonfrom^3.4.14to^3.4.17. Issue#8730provides no additional bug details, acceptance criteria, or test requirements.
Full details: Out of Scope Changes check
-
Explanation
The reviewed change is limited to the
dompurifydependency declaration inspecifyweb/frontend/js_src/package.json. This change directly supports issue#8730. No unrelated change is shown.package-lock.jsonis intentionally excluded from review, so its contents are not assessed.
Full details: Automatic Tests
-
Explanation
PASS. The PR only updates the DOMPurify version in package.json and package-lock.json. It does not change application logic or introduce a new behavior that requires an automatic test. The frontend already provides an automatic test command through the existing
npm testscript.
-
Full details: Testing Instructions
-
Explanation
The PR changes the frontend dependency and lockfile from DOMPurify 3.4.14 to 3.4.17, but the PR description provides no testing steps. The affected component is
specifyweb/frontend/js_src; its package definesnpm testfor type checking, unit tests, and localization tests. The testing section contains only an empty placeholder, so it does not clearly cover the changed frontend dependency.Resolution
Add clear testing instructions. State that commands run from
specifyweb/frontend/js_src, install the locked dependencies withnpm ci, runnpm test, and perform a frontend smoke test of the DOM-sanitization paths that use DOMPurify. Record the result and any relevant environment details.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
g1rly-c0d3r
left a comment
g1rly-c0d3r
left a comment
There was a problem hiding this comment.
Testing instructions
- Some workbench general testing
Looks good! I did not notice any regressions.
rijulpoudel
left a comment
rijulpoudel
left a comment
There was a problem hiding this comment.
Testing instructions
- Some workbench general testing
Everything seems to be working good as before.
There was a problem hiding this comment.
Testing instructions
- Some workbench general testing
When I use the Fill Up button it seems to not update the first two rows in the column. This behavior seems to be present in v7.12.2, so I'm not sure if it's an error or the expected behavior.
Live.Validation.Errors.mp4
Live validation doesn't seem to actually remove errors without selecting Validate manually. This is not in v7.12.2, so it is likely an issue.
github-project-automation
Bot
moved this from 📋Back Log
to Dev Attention Needed
in General Tester Board
JDAM2k4
left a comment
JDAM2k4
left a comment
There was a problem hiding this comment.
Updating my review to a comment, as it seems this behavior is present in v7.12.2.
github-project-automation
Bot
moved this from Dev Attention Needed
to ✅Done
in General Tester Board
Fixes #8730
Checklist
self-explanatory (or properly documented)
specify7/specifyweb/specify/management/commands/run_key_migration_functions.py
Line 50 in ea04665
Testing instructions
Summary by CodeRabbit