Skip to content

[Required brand] decorators: add missing arguments on beforeModel#15

Merged
aprentout merged 1 commit into
mainfrom
ap/fix-missing-params
Apr 30, 2026
Merged

[Required brand] decorators: add missing arguments on beforeModel#15
aprentout merged 1 commit into
mainfrom
ap/fix-missing-params

Conversation

@aprentout

Copy link
Copy Markdown
Contributor

What does this PR do?

Related to: #DRA-5001

What are the observable changes?

The goal of the PR is to avoid loosing beforeModel arguments when the required-brand decorator is used

Good PR checklist

  • Title makes sense
  • Is against the correct branch
  • Only addresses one issue
  • Properly assigned
  • Added/updated tests
  • Added/updated documentation
  • Properly labeled

@aprentout aprentout self-assigned this Apr 29, 2026
@aprentout aprentout requested review from a team, Miexil and phndiaye as code owners April 29, 2026 16:11
@aprentout aprentout requested review from edouardmisset and removed request for a team April 29, 2026 16:11
Comment thread addon/decorators/required-brand.ts Outdated
Comment on lines +16 to +28
@@ -24,9 +25,9 @@ export function requiredBrand(brand: string, fallbackRoute: string) {
if ((getOwnConfig() as any).brand !== brand) {
this.router.transitionTo(fallbackRoute);
} else {
super.beforeModel()
super.beforeModel(...arguments);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

question: did the parameter transform from transition to …arguments ? I'm probably missing something :/

@aprentout aprentout force-pushed the ap/fix-missing-params branch from 31793de to d31ea84 Compare April 29, 2026 16:15
@aprentout aprentout merged commit fac05a2 into main Apr 30, 2026
1 check passed
@aprentout aprentout deleted the ap/fix-missing-params branch April 30, 2026 09:59
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.

5 participants