Skip to content

fix(contributions): auto-redirect vers la concention collective quand celle-ci a été sauvegardée dans le header#7203

Open
maxgfr wants to merge 2 commits intodevfrom
maxgfr/auto-redirect
Open

fix(contributions): auto-redirect vers la concention collective quand celle-ci a été sauvegardée dans le header#7203
maxgfr wants to merge 2 commits intodevfrom
maxgfr/auto-redirect

Conversation

@maxgfr
Copy link
Copy Markdown
Member

@maxgfr maxgfr commented Mar 27, 2026

fix #7182

@revu-bot revu-bot bot requested a review from revu-bot March 27, 2026 16:18
@maxgfr maxgfr temporarily deployed to build-preproduction March 27, 2026 16:18 — with GitHub Actions Inactive
Copy link
Copy Markdown
Collaborator

@revu-bot revu-bot left a comment

Choose a reason for hiding this comment

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

PR Review: Auto-redirect to collective agreement page from header

This PR implements auto-redirection to a CC-specific contribution page when a collective agreement has been previously saved in localStorage (via the site header). The implementation is generally clean and well-tested, but there are a few issues worth addressing.

File Lines Severity Issue
ContributionGeneric.tsx 58–71 CRITICAL Missing redirect loop guard — unpublished CC can cause infinite redirect
ContributionGeneric.tsx 58–71 IMPORTANT useEffect with empty deps array accesses stale slug/contribution via closure
ContributionGeneric.tsx 63–65 IMPORTANT Hardcoded slug string is a fragile magic constant
contributions.test.tsx 308–385 IMPORTANT Tests don't clear localStorage between cases — state leaks between tests

Comment on lines +65 to +68
const targetUrl =
slug === "les-conges-pour-evenements-familiaux"
? `/contribution/${slug}/${storedAgreement.slug || storedAgreement.num}`
: `/contribution/${storedAgreement.num}-${slug}`;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[IMPORTANT] Hardcoded slug string is a fragile magic constant

  • If the slug for "congés pour événements familiaux" ever changes (rename, typo fix, etc.), this silent branch will silently stop working with no compile-time error.
  • The special-casing logic is duplicated here and likely elsewhere in the codebase.

Proposed fix: Extract this to a named constant (or derive it from the contribution data) so the intent is clear and the value is maintained in one place.

Suggested change
const targetUrl =
slug === "les-conges-pour-evenements-familiaux"
? `/contribution/${slug}/${storedAgreement.slug || storedAgreement.num}`
: `/contribution/${storedAgreement.num}-${slug}`;
const CONGES_FAMILIAUX_SLUG = "les-conges-pour-evenements-familiaux";
const targetUrl =
slug === CONGES_FAMILIAUX_SLUG
? `/contribution/${slug}/${storedAgreement.slug || storedAgreement.num}`
: `/contribution/${storedAgreement.num}-${slug}`;

@maxgfr maxgfr deployed to build-review-auto March 27, 2026 16:32 — with GitHub Actions Active
@maxgfr maxgfr deployed to build-preproduction March 27, 2026 16:32 — with GitHub Actions Active
@sonarqubecloud
Copy link
Copy Markdown

@maxgfr maxgfr deployed to review-auto March 27, 2026 16:38 — with GitHub Actions Active
@tokenbureau
Copy link
Copy Markdown

tokenbureau bot commented Mar 27, 2026

@maxgfr maxgfr requested a review from m-maillot March 27, 2026 17:25
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.

CC header - niveau 2

2 participants