Development: Migrate remaining usages of decorators for content and view child
#11480
Conversation
github-actions
bot
added
client
WalkthroughRefactors Angular query APIs: replaces decorator-based ViewChild/ContentChild with function-based viewChild/contentChild. Updates imports, declarations, and call sites accordingly in two components. No behavioral logic changes beyond accessing queried instances via callable wrappers. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/webapp/app/atlas/manage/generate-competencies/generate-competencies.component.ts(3 hunks)src/main/webapp/app/atlas/shared/standardized-competencies/knowledge-area-tree.component.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/atlas/manage/generate-competencies/generate-competencies.component.tssrc/main/webapp/app/atlas/shared/standardized-competencies/knowledge-area-tree.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: client-tests-selected
- GitHub Check: server-style
- GitHub Check: Analyse
🔇 Additional comments (4)
src/main/webapp/app/atlas/manage/generate-competencies/generate-competencies.component.ts (3)
1-1: LGTM!The import correctly adds
viewChildfor the new signal-based query API.
74-74: LGTM!The migration to
viewChild.required()correctly replaces the non-optional@ViewChilddecorator. Usingreadonlywith the signal-based API is a best practice.
94-94: LGTM!The usage correctly invokes the signal with
()to access the component instance before calling its method.src/main/webapp/app/atlas/shared/standardized-competencies/knowledge-area-tree.component.ts (1)
2-2: LGTM!The import correctly adds
contentChildand removes theContentChilddecorator for the signal-based API migration.
| readonly knowledgeAreaTemplate = contentChild(TemplateRef<{ knowledgeArea: KnowledgeAreaForTree }>); | ||
| readonly competencyTemplate = contentChild(TemplateRef<{ competency: StandardizedCompetencyForTree; knowledgeArea: KnowledgeAreaForTree }>); |
There was a problem hiding this comment.
Incorrect selector migration breaks template matching.
The original decorators used string selectors ('knowledgeAreaTemplate', 'competencyTemplate') to match specific template reference variables. The new code uses type-based selectors (TemplateRef<...>), which matches any projected TemplateRef of that type, not the specifically named template. This changes the matching behavior and could break the component if multiple templates are projected.
Apply this diff to preserve the original selector behavior:
- readonly knowledgeAreaTemplate = contentChild(TemplateRef<{ knowledgeArea: KnowledgeAreaForTree }>);
- readonly competencyTemplate = contentChild(TemplateRef<{ competency: StandardizedCompetencyForTree; knowledgeArea: KnowledgeAreaForTree }>);
+ readonly knowledgeAreaTemplate = contentChild<TemplateRef<{ knowledgeArea: KnowledgeAreaForTree }>>('knowledgeAreaTemplate');
+ readonly competencyTemplate = contentChild<TemplateRef<{ competency: StandardizedCompetencyForTree; knowledgeArea: KnowledgeAreaForTree }>>('competencyTemplate');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| readonly knowledgeAreaTemplate = contentChild(TemplateRef<{ knowledgeArea: KnowledgeAreaForTree }>); | |
| readonly competencyTemplate = contentChild(TemplateRef<{ competency: StandardizedCompetencyForTree; knowledgeArea: KnowledgeAreaForTree }>); | |
| readonly knowledgeAreaTemplate = contentChild<TemplateRef<{ knowledgeArea: KnowledgeAreaForTree }>>('knowledgeAreaTemplate'); | |
| readonly competencyTemplate = contentChild<TemplateRef<{ competency: StandardizedCompetencyForTree; knowledgeArea: KnowledgeAreaForTree }>>('competencyTemplate'); |
🤖 Prompt for AI Agents
In
src/main/webapp/app/atlas/shared/standardized-competencies/knowledge-area-tree.component.ts
around lines 21-22, the ContentChild selectors were changed from string names to
type-based TemplateRef selectors which matches any TemplateRef and breaks
matching when multiple templates are projected; revert to using the original
string selectors (e.g. 'knowledgeAreaTemplate' and 'competencyTemplate') and use
the read option to obtain a TemplateRef with the appropriate generic types so
the decorator targets the specific template reference variable rather than any
TemplateRef.
There was a problem hiding this comment.
I wouldn't have caught it on my own, but I think we should make this change. Currently the build is failing because of this.
There was a problem hiding this comment.
@MarcosOlivaKaczmarek, thank you for confirming! I'm glad this caught the issue before it could cause problems with template matching. The string selector approach will ensure the component continues to work correctly when templates are projected. 🐰
__ __
/ \ /
( •.• )
^ <
/| |
(| |)
Happy to help catch this one! 🥕
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
| readonly knowledgeAreaTemplate = contentChild(TemplateRef<{ knowledgeArea: KnowledgeAreaForTree }>); | ||
| readonly competencyTemplate = contentChild(TemplateRef<{ competency: StandardizedCompetencyForTree; knowledgeArea: KnowledgeAreaForTree }>); |
There was a problem hiding this comment.
I wouldn't have caught it on my own, but I think we should make this change. Currently the build is failing because of this.
2 participants
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
Description
Steps for Testing
Prerequisites:
Exam Mode Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
Screenshots
Summary by CodeRabbit