Fixes CodeQL findings exposed by headers - #24463
Open
myabc wants to merge 4 commits into
Open
Conversation
Turns the JS and TS tasks from reporters into writers so headers can be repaired in bulk. The matcher accepts the header shapes actually in the tree, and skips vendored bundles, caches and build output, which are generated and must not be rewritten. https://community.openproject.org/wp/OP-19821
Brings existing first-party sources into the canonical form before strict lint enforcement is enabled. Reproduce with: bundle exec rake copyright:update_js bundle exec rake copyright:update_typescript https://community.openproject.org/wp/OP-19821
Lints the canonical header so it cannot drift again. Both generators of linked-plugins.module.ts emit it from COPYRIGHT_short, since that file is not hand-editable, and gitlab_integration stays exempt to preserve its third-party notice. https://community.openproject.org/wp/OP-19821
myabc
force-pushed
the
code-maintenance/OP-19821-normalize-ts-js-copyright-headers
branch
from
July 28, 2026 10:56
5e9e953 to
b5c72b7
Compare
Header-only line shifts surfaced pre-existing unused code and two static-analysis defects. Clearing them keeps follow-up remediation separate from the copyright-header change.
myabc
force-pushed
the
housekeeping/codeql-issue-follow-up
branch
from
July 28, 2026 11:06
158f7cf to
e92eb8e
Compare
myabc
marked this pull request as ready for review
July 28, 2026 11:06
|
Warning This pull request does not link an OpenProject work package. Please add a link to the work package in the description, or reference it in the |
myabc
changed the title
3 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses CodeQL/static-analysis findings surfaced after header-only changes by removing unused code/imports and fixing filter-string escaping/serialization to avoid defects in the dynamic filter form.
Changes:
- Introduced
escapeFilterValue()and added a unit spec to ensure correct escaping of backslashes and quotes in filter serialization. - Removed multiple unused imports/locals across Angular/Stimulus code and specs to clear static-analysis warnings.
- Tweaked scroll-sync flag reset logic in work package table timeline scrolling.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.ts | Adds escapeFilterValue() and uses it when building the serialized filters query param. |
| frontend/src/stimulus/controllers/dynamic/filter/filters-form.controller.spec.ts | New unit spec for escapeFilterValue(). |
| frontend/src/app/shared/components/toaster/toast.service.spec.ts | Removes unused locals in specs. |
| frontend/src/app/shared/components/grids/widgets/add/add.modal.ts | Removes unused enterpriseDocsUrl import. |
| frontend/src/app/shared/components/fields/openproject-fields.module.ts | Removes unused Injector import. |
| frontend/src/app/shared/components/fields/changeset/resource-changeset.ts | Removes unused imports/types. |
| frontend/src/app/features/work-packages/routing/work-packages-routes.ts | Removes unused route-related imports. |
| frontend/src/app/features/work-packages/components/wp-table/wp-table-scroll-sync.ts | Adjusts scroll-sync state reset condition. |
| frontend/src/app/features/work-packages/components/wp-table/timeline/global-elements/wp-timeline-relations.directive.ts | Removes unused imports/operators. |
| frontend/src/app/features/work-packages/components/wp-query/url-params-helper.spec.ts | Removes unused local declaration. |
| frontend/src/app/features/work-packages/components/wp-new/wp-create.component.ts | Removes unused imports. |
| frontend/src/app/features/work-packages/components/wp-fast-table/builders/cell-builder.ts | Removes unused type import. |
| frontend/src/app/features/work-packages/components/wp-baseline/baseline/baseline.component.ts | Removes unused enterpriseDocsUrl import. |
| frontend/src/app/features/team-planner/team-planner/planner/team-planner.component.ts | Removes unused icon helper import. |
Comment on lines
511
to
515
| private buildFilterString(filter:InternalFilterValue) { | ||
| const valuesString = filter.value.length > 1 ? `[${filter.value.map((v) => `"${this.replaceDoubleQuotes(v)}"`).join(',')}]` : `"${this.replaceDoubleQuotes(filter.value[0])}"`; | ||
| const valuesString = filter.value.length > 1 ? `[${filter.value.map((v) => `"${escapeFilterValue(v)}"`).join(',')}]` : `"${escapeFilterValue(filter.value[0])}"`; | ||
|
|
||
| return `${filter.name} ${filter.operator} ${valuesString}`; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This behaviour is unchanged by this PR: the previous replaceDoubleQuotes also returned '' for filter.value[0] === undefined, so name op "" was already emitted for value-less operators. The diff here is a rename + backslash escaping.
myabc
force-pushed
the
code-maintenance/OP-19821-normalize-ts-js-copyright-headers
branch
from
July 28, 2026 22:11
b5c72b7 to
4949534
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
n/a
What are you trying to accomplish?
Header-only line shifts surfaced pre-existing unused code and two static-analysis defects. Clearing them keeps follow-up remediation separate from the copyright-header change.
Screenshots
What approach did you choose and why?
Merge checklist