[NAE-2217] Single Task View #303
Conversation
- move task list and padination to new folder - add new injection token to set a task content service - rework of panel for be able to work with multiple tasks for single task view
- remove check id
WalkthroughRefactors TaskPanelData location and updates imports. Introduces TaskContentServiceType enum, DI token, and a factory to instantiate Single/Unlimited task content services. Adjusts providers in several components/tests to select service type. Enhances AbstractTaskPanel data setter for Unlimited service. Reorganizes public API exports. Adds transitionId-based filter logic in a tabbed single-task view. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Component (Task panel host)
participant DI as Angular Injector
participant F as TaskContentServiceFactory
participant S as SingleTaskContentService
participant U as UnlimitedTaskContentService
C->>DI: Resolve TaskContentService<br/>(token: NAE_TASK_CONTENT_SERVICE_TYPE)
DI-->>C: Injected type (SINGLE or UNLIMITED)
C->>F: create service by type
alt Type = SINGLE
F-->>C: S instance
else Type = UNLIMITED
F-->>C: U instance
end
note over C: Service injected via provider useFactory
sequenceDiagram
autonumber
participant Caller
participant P as AbstractTaskPanel
participant U as UnlimitedTaskContentService
participant R as panelRef
Caller->>P: set taskPanelData(newData)
alt Service is Unlimited and panelRef exists
P->>R: collapse()
P->>U: setTask(newData.task)
P->>U: unsubscribe previous changedFields
U-->>P: changedFields$
P->>P: subscribe -> updateFromChangedFields(...)
P->>R: expand()
end
P->>P: resolveFeaturedFieldsValues(newData)
sequenceDiagram
autonumber
actor User
participant V as DefaultTabbedSingleTaskView
participant BF as baseFilterFactory
User->>V: Navigate to tab
V->>BF: build filter (requestBody, navigationItemTaskData)
alt transitionId missing in requestBody and present in navigation data
BF-->>V: base AND TASK SimpleFilter(transitionId[])
else
BF-->>V: base filter unchanged
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/netgrif-components-core/src/lib/panel/public-api.ts (1)
1-20: Restore or refactor removed TaskList exports.
- projects/netgrif-components-core/src/lib/view/public-api.ts (lines 62–63) still reference
abstract-task-list.componentandabstract-task-list-pagination.component; their removal breaksTaskListPaginationComponentin projects/netgrif-components/src/lib/view/task-view/task-panel-list-pagination and its spec.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (31)
projects/netgrif-components-core/src/lib/panel/public-api.ts(1 hunks)projects/netgrif-components-core/src/lib/panel/task-panel-data/task-panel-data.ts(1 hunks)projects/netgrif-components-core/src/lib/panel/task-panel-single/abstract-single-task.component.spec.ts(1 hunks)projects/netgrif-components-core/src/lib/panel/task-panel-single/abstract-single-task.component.ts(1 hunks)projects/netgrif-components-core/src/lib/panel/task-panel/abstract-task-panel.component.spec.ts(1 hunks)projects/netgrif-components-core/src/lib/panel/task-panel/abstract-task-panel.component.ts(3 hunks)projects/netgrif-components-core/src/lib/task-content/model/task-content-injection-token.ts(1 hunks)projects/netgrif-components-core/src/lib/task-content/model/task-content-service-type.ts(1 hunks)projects/netgrif-components-core/src/lib/task-content/public-api.ts(2 hunks)projects/netgrif-components-core/src/lib/task-content/services/task-content-service-factory.ts(1 hunks)projects/netgrif-components-core/src/lib/view/public-api.ts(1 hunks)projects/netgrif-components-core/src/lib/view/task-view/abstract-single-task-view.component.ts(1 hunks)projects/netgrif-components-core/src/lib/view/task-view/abstract-task-view.ts(1 hunks)projects/netgrif-components-core/src/lib/view/task-view/components/default-task-panel-list/abstract-default-task-list.component.ts(1 hunks)projects/netgrif-components-core/src/lib/view/task-view/components/task-panel-list-pagination/abstract-task-list-pagination.component.ts(1 hunks)projects/netgrif-components-core/src/lib/view/task-view/components/task-panel-list/abstract-task-list.component.spec.ts(1 hunks)projects/netgrif-components-core/src/lib/view/task-view/components/task-panel-list/abstract-task-list.component.ts(1 hunks)projects/netgrif-components-core/src/lib/view/task-view/models/task-page-load-request-result.ts(1 hunks)projects/netgrif-components-core/src/lib/view/task-view/service/task-view.service.spec.ts(1 hunks)projects/netgrif-components-core/src/lib/view/task-view/service/task-view.service.ts(1 hunks)projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/tabbed/default-tabbed-single-task-view/default-tabbed-single-task-view.component.ts(1 hunks)projects/netgrif-components/src/lib/panel/panel.module.ts(2 hunks)projects/netgrif-components/src/lib/panel/public-api.ts(0 hunks)projects/netgrif-components/src/lib/panel/task-panel-single/single-task.component.ts(1 hunks)projects/netgrif-components/src/lib/panel/task-panel/task-panel.component.spec.ts(2 hunks)projects/netgrif-components/src/lib/panel/task-panel/task-panel.component.ts(1 hunks)projects/netgrif-components/src/lib/view/public-api.ts(1 hunks)projects/netgrif-components/src/lib/view/task-view/task-panel-list-pagination/task-list-pagination.component.spec.ts(2 hunks)projects/netgrif-components/src/lib/view/task-view/task-panel-list-pagination/task-list-pagination.component.ts(2 hunks)projects/netgrif-components/src/lib/view/task-view/task-panel-list/task-list.component.spec.ts(1 hunks)projects/netgrif-components/src/lib/view/task-view/task-panel-list/task-list.component.ts(1 hunks)
💤 Files with no reviewable changes (1)
- projects/netgrif-components/src/lib/panel/public-api.ts
🧰 Additional context used
🧬 Code graph analysis (6)
projects/netgrif-components/src/lib/panel/task-panel-single/single-task.component.ts (4)
projects/netgrif-components/src/lib/panel/task-panel/task-panel.component.ts (1)
Component(47-117)projects/netgrif-components/src/lib/view/task-view/task-panel-list-pagination/task-list-pagination.component.ts (1)
Component(17-33)projects/netgrif-components/src/lib/view/task-view/task-panel-list/task-list.component.ts (1)
Component(12-25)projects/netgrif-components-core/src/lib/task-content/model/task-content-injection-token.ts (1)
NAE_TASK_CONTENT_SERVICE_TYPE(4-4)
projects/netgrif-components/src/lib/panel/task-panel/task-panel.component.spec.ts (1)
projects/netgrif-components-core/src/lib/task-content/model/task-content-injection-token.ts (1)
NAE_TASK_CONTENT_SERVICE_TYPE(4-4)
projects/netgrif-components/src/lib/view/task-view/task-panel-list/task-list.component.ts (4)
projects/netgrif-components/src/lib/panel/task-panel-single/single-task.component.ts (1)
Component(11-25)projects/netgrif-components/src/lib/panel/task-panel/task-panel.component.ts (1)
Component(47-117)projects/netgrif-components/src/lib/view/task-view/task-panel-list-pagination/task-list-pagination.component.ts (1)
Component(17-33)projects/netgrif-components-core/src/lib/task-content/model/task-content-injection-token.ts (1)
NAE_TASK_CONTENT_SERVICE_TYPE(4-4)
projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/tabbed/default-tabbed-single-task-view/default-tabbed-single-task-view.component.ts (3)
projects/netgrif-components-core/src/lib/filter/models/task-search-request-body.ts (1)
TaskSearchRequestBody(9-80)projects/netgrif-components-core/src/lib/resources/interface/data-groups.ts (1)
DataGroup(7-28)projects/netgrif-components-core/src/lib/navigation/utility/navigation-item-task-utility-methods.ts (1)
extractFieldValueFromData(127-133)
projects/netgrif-components/src/lib/view/task-view/task-panel-list-pagination/task-list-pagination.component.ts (1)
projects/netgrif-components-core/src/lib/task-content/model/task-content-injection-token.ts (1)
NAE_TASK_CONTENT_SERVICE_TYPE(4-4)
projects/netgrif-components/src/lib/panel/task-panel/task-panel.component.ts (4)
projects/netgrif-components/src/lib/panel/task-panel-single/single-task.component.ts (1)
Component(11-25)projects/netgrif-components/src/lib/view/task-view/task-panel-list-pagination/task-list-pagination.component.ts (1)
Component(17-33)projects/netgrif-components/src/lib/view/task-view/task-panel-list/task-list.component.ts (1)
Component(12-25)projects/netgrif-components-core/src/lib/task-content/model/task-content-injection-token.ts (1)
NAE_TASK_CONTENT_SERVICE_TYPE(4-4)
⏰ 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). (6)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Test with SonarCloud
- GitHub Check: Matrix Test (24)
- GitHub Check: Matrix Test (22)
- GitHub Check: Matrix Test (20)
🔇 Additional comments (30)
projects/netgrif-components-core/src/lib/view/task-view/models/task-page-load-request-result.ts (1)
1-1: Import path relocation confirmedThe updated path matches the new
panel/task-panel-datalocation; no further action needed.projects/netgrif-components-core/src/lib/view/public-api.ts (1)
62-63: Approve addition of abstract task list exports
Files exist at the specified paths, define abstract components, and no prior exports of these classes were detected—additive change.projects/netgrif-components-core/src/lib/view/task-view/components/task-panel-list-pagination/abstract-task-list-pagination.component.ts (1)
6-10: LGTM! Clean import path refactor.The import paths have been correctly updated to reference the centralized
TaskPanelDatalocation. The relative paths are accurate for this file's location in the directory structure.projects/netgrif-components-core/src/lib/view/task-view/components/default-task-panel-list/abstract-default-task-list.component.ts (1)
4-13: LGTM! Import paths correctly updated.All import paths have been properly updated to reflect the new centralized module locations. The relative path calculations are correct for this file's position in the directory hierarchy.
projects/netgrif-components-core/src/lib/view/task-view/abstract-single-task-view.component.ts (1)
4-4: LGTM! Import path correctly updated.The
TaskPanelDataimport path has been properly updated to reference the centralized location. The relative path is accurate for this file's location.projects/netgrif-components-core/src/lib/panel/task-panel-single/abstract-single-task.component.spec.ts (1)
32-32: LGTM! Test import updated correctly.The import path for
TaskPanelDatain the test spec has been correctly updated to reference the centralized location. No test logic changes.projects/netgrif-components-core/src/lib/panel/task-panel-single/abstract-single-task.component.ts (1)
3-3: LGTM! Import path correctly updated.The
TaskPanelDataimport has been properly updated to use the new centralized location. The relative path is accurate.projects/netgrif-components-core/src/lib/panel/task-panel/abstract-task-panel.component.spec.ts (1)
39-39: LGTM! Test import updated correctly.The import path for
TaskPanelDatahas been correctly updated in the test spec to reference the centralized module location.projects/netgrif-components-core/src/lib/view/task-view/service/task-view.service.ts (1)
3-3: LGTM! Service import correctly updated.The
TaskPanelDataimport path has been properly updated to reference the centralized location. The relative path calculation is correct for this service file's location.projects/netgrif-components-core/src/lib/view/task-view/service/task-view.service.spec.ts (1)
24-24: All TaskPanelData imports and exports verifiedTaskPanelData exists at projects/netgrif-components-core/src/lib/panel/task-panel-data/task-panel-data.ts, is exported in panel/public-api.ts, and no obsolete import paths remain. Please run a full build and test suite to confirm compilation succeeds and no circular dependencies were introduced.
projects/netgrif-components-core/src/lib/view/task-view/components/task-panel-list/abstract-task-list.component.ts (1)
3-10: LGTM! Import path reorganization correctly applied.All import paths have been properly updated to reflect the new module structure. No behavioral changes detected.
projects/netgrif-components-core/src/lib/panel/task-panel-data/task-panel-data.ts (1)
1-3: LGTM! Import paths correctly adjusted for file relocation.The import paths have been properly updated to reflect the file's new location in the directory hierarchy.
projects/netgrif-components-core/src/lib/view/task-view/components/task-panel-list/abstract-task-list.component.spec.ts (1)
10-31: LGTM! Test imports correctly updated.Test file imports have been properly synchronized with the source file reorganization.
projects/netgrif-components-core/src/lib/view/task-view/abstract-task-view.ts (1)
3-3: LGTM! Import path updated to centralized TaskPanelData location.The import path correctly reflects the relocation of TaskPanelData to a centralized module.
projects/netgrif-components-core/src/lib/panel/task-panel/abstract-task-panel.component.ts (2)
18-18: LGTM! Import path correctly updated.The TaskPanelData import path has been properly adjusted to the centralized location.
56-56: New import added for dynamic task switching support.The UnlimitedTaskContentService import supports the new dynamic task switching logic in the setter below.
projects/netgrif-components-core/src/lib/task-content/model/task-content-service-type.ts (1)
1-10: LGTM!The
TaskContentServiceTypeenum is well-defined with clear documentation for each member. This provides a clean type-safe way to distinguish between single and unlimited task content service implementations.projects/netgrif-components/src/lib/view/task-view/task-panel-list-pagination/task-list-pagination.component.spec.ts (1)
4-4: LGTM!The test import paths have been correctly updated to match the new directory structure. The test logic remains unchanged.
Also applies to: 25-25
projects/netgrif-components/src/lib/panel/task-panel-single/single-task.component.ts (1)
15-15: Verify the UNLIMITED service type choice is appropriate.The
SingleTaskComponentis configured to useTaskContentServiceType.UNLIMITED, while list-based components (e.g.,TaskListComponent,TaskListPaginationComponent) useSINGLE. Confirm this distinction aligns with the intended behavior: unlimited allows multiple concurrent task content instances within a single view, whereas single restricts to one task content instance per component.projects/netgrif-components-core/src/lib/task-content/model/task-content-injection-token.ts (1)
1-4: LGTM!The injection token is properly defined with the correct type annotation. This follows Angular dependency injection best practices.
projects/netgrif-components-core/src/lib/task-content/public-api.ts (1)
7-7: LGTM!The public API correctly exposes the new task content service infrastructure (factory, enum, and injection token), enabling external consumers to leverage the SINGLE/UNLIMITED service type system.
Also applies to: 21-22
projects/netgrif-components/src/lib/view/task-view/task-panel-list/task-list.component.ts (1)
16-16: LGTM!The
TaskListComponentis correctly configured withTaskContentServiceType.SINGLE, consistent with other list-based components. This ensures each task in the list maintains isolated task content state.projects/netgrif-components/src/lib/panel/panel.module.ts (1)
7-7: Import paths verified
Both TaskListComponent and TaskListPaginationComponent exist at their updated locations.projects/netgrif-components/src/lib/view/public-api.ts (1)
8-9: Confirm public API exports The new exports align with the reorganized file structure and the component files exist; verify they should be exposed in the public API surface.projects/netgrif-components/src/lib/view/task-view/task-panel-list-pagination/task-list-pagination.component.ts (1)
21-24: LGTM! Service type provider correctly configured.The provider for
NAE_TASK_CONTENT_SERVICE_TYPEwith valueSINGLEis appropriate for a list/pagination component where only one task should be active at a time. This aligns with the usage pattern seen in other list components.projects/netgrif-components/src/lib/panel/task-panel/task-panel.component.spec.ts (1)
114-115: LGTM! Test provider correctly configured.The test correctly provides
NAE_TASK_CONTENT_SERVICE_TYPEwith valueSINGLE, which is required for the factory-basedTaskContentServiceprovider inTaskPanelComponentto function. This aligns with the component's DI requirements.projects/netgrif-components-core/src/lib/task-content/services/task-content-service-factory.ts (1)
9-27: LGTM! Factory pattern correctly implemented.The
TaskContentServiceFactoryis well-structured and follows Angular best practices:
- Properly scoped as a root-level singleton
- Each factory method creates new service instances with correctly injected dependencies
- Clean delegation pattern that centralizes service instantiation logic
- Enables runtime selection of service implementations via DI
projects/netgrif-components/src/lib/panel/task-panel/task-panel.component.ts (2)
40-45: LGTM!The factory function correctly selects between single and unlimited task content services based on the provided type. Module-level definition is appropriate.
52-52: Verify that required dependencies are available in the DI tree.The factory depends on
NAE_TASK_CONTENT_SERVICE_TYPEandTaskContentServiceFactory, but neither is provided in this component's providers array. This component now requires these dependencies to be available from parent components or at module level, which is a behavioral change.Ensure that:
- All uses of
TaskPanelComponenthaveNAE_TASK_CONTENT_SERVICE_TYPEprovided in their DI treeTaskContentServiceFactoryis registered at module level- Existing standalone uses of this component won't break with missing provider errors
Run the following script to check how
TaskPanelComponentis used throughout the codebase:projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/tabbed/default-tabbed-single-task-view/default-tabbed-single-task-view.component.ts (1)
42-42: Ignore empty-stringidin SimpleFilter constructor — intended usage
Passing''as theidparameter is valid and used throughout the codebase and tests as a placeholder. No change required.Likely an incorrect or invalid review comment.
| if (this._taskContentService instanceof UnlimitedTaskContentService && this.panelRef) { | ||
| this.collapse(); | ||
| this._taskContentService.task = this._taskPanelData.task; | ||
| if (this._sub) { | ||
| this._sub.unsubscribe(); | ||
| } | ||
| this._sub = this._taskPanelData.changedFields.subscribe(chFields => { | ||
| this._taskContentService.updateFromChangedFields(chFields); | ||
| }); | ||
| this.expand(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add documentation for the UnlimitedTaskContentService special handling.
The logic correctly handles dynamic task switching for UnlimitedTaskContentService by collapsing the panel, updating the task, properly managing subscriptions, and re-expanding. However, the purpose and rationale for this special handling should be documented.
Consider adding a JSDoc comment or inline comment explaining:
- Why UnlimitedTaskContentService requires collapse/expand during task switches
- Whether the collapse/expand sequence is necessary for cleaning up state or preventing UI issues
- Any side effects or timing considerations developers should be aware of
Example:
@Input()
public set taskPanelData(data: TaskPanelData) {
this._taskPanelData = data;
+ // UnlimitedTaskContentService allows dynamic task switching within the same panel.
+ // Collapse and re-expand to properly reset panel state and trigger lifecycle hooks.
if (this._taskContentService instanceof UnlimitedTaskContentService && this.panelRef) {
this.collapse();📝 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.
| if (this._taskContentService instanceof UnlimitedTaskContentService && this.panelRef) { | |
| this.collapse(); | |
| this._taskContentService.task = this._taskPanelData.task; | |
| if (this._sub) { | |
| this._sub.unsubscribe(); | |
| } | |
| this._sub = this._taskPanelData.changedFields.subscribe(chFields => { | |
| this._taskContentService.updateFromChangedFields(chFields); | |
| }); | |
| this.expand(); | |
| } | |
| @Input() | |
| public set taskPanelData(data: TaskPanelData) { | |
| this._taskPanelData = data; | |
| // UnlimitedTaskContentService allows dynamic task switching within the same panel. | |
| // Collapse and re-expand to properly reset panel state and trigger lifecycle hooks. | |
| if (this._taskContentService instanceof UnlimitedTaskContentService && this.panelRef) { | |
| this.collapse(); | |
| this._taskContentService.task = this._taskPanelData.task; | |
| if (this._sub) { | |
| this._sub.unsubscribe(); | |
| } | |
| this._sub = this._taskPanelData.changedFields.subscribe(chFields => { | |
| this._taskContentService.updateFromChangedFields(chFields); | |
| }); | |
| this.expand(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/panel/task-panel/abstract-task-panel.component.ts
around lines 261 to 271, add a concise JSDoc or inline comment above the
UnlimitedTaskContentService special-case block explaining why collapse() and
expand() are required when switching tasks: state that
UnlimitedTaskContentService preserves internal UI/state tied to a task so
collapsing ensures teardown of UI/state and unsubscribes, updating the
service.task replaces internal references, re-subscribing to changedFields must
be done after teardown to avoid stale handlers, and note any timing or
side-effect considerations (e.g., animations or async cleanup) so future
maintainers understand the rationale and when it can be safely changed.
| public createSingleTaskContentService(): SingleTaskContentService { | ||
| return new SingleTaskContentService(this._fieldConverterService, this._snackBarService, this._translate, this._log, this._injector); | ||
| } | ||
|
|
||
| public createUnlimitedTaskContentService(): UnlimitedTaskContentService { | ||
| return new UnlimitedTaskContentService(this._fieldConverterService, this._snackBarService, this._translate, this._log, this._injector); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding JSDoc for public factory methods.
The factory methods are well-implemented and follow Angular patterns. Consider adding JSDoc comments to document when each service type should be used:
+ /**
+ * Creates a SingleTaskContentService instance.
+ * Use for components that should display only one task at a time (e.g., task lists).
+ */
public createSingleTaskContentService(): SingleTaskContentService {
return new SingleTaskContentService(this._fieldConverterService, this._snackBarService, this._translate, this._log, this._injector);
}
+ /**
+ * Creates an UnlimitedTaskContentService instance.
+ * Use for components that can display multiple tasks simultaneously (e.g., single task views).
+ */
public createUnlimitedTaskContentService(): UnlimitedTaskContentService {
return new UnlimitedTaskContentService(this._fieldConverterService, this._snackBarService, this._translate, this._log, this._injector);
}📝 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.
| public createSingleTaskContentService(): SingleTaskContentService { | |
| return new SingleTaskContentService(this._fieldConverterService, this._snackBarService, this._translate, this._log, this._injector); | |
| } | |
| public createUnlimitedTaskContentService(): UnlimitedTaskContentService { | |
| return new UnlimitedTaskContentService(this._fieldConverterService, this._snackBarService, this._translate, this._log, this._injector); | |
| } | |
| /** | |
| * Creates a SingleTaskContentService instance. | |
| * Use for components that should display only one task at a time (e.g., task lists). | |
| */ | |
| public createSingleTaskContentService(): SingleTaskContentService { | |
| return new SingleTaskContentService(this._fieldConverterService, this._snackBarService, this._translate, this._log, this._injector); | |
| } | |
| /** | |
| * Creates an UnlimitedTaskContentService instance. | |
| * Use for components that can display multiple tasks simultaneously (e.g., single task views). | |
| */ | |
| public createUnlimitedTaskContentService(): UnlimitedTaskContentService { | |
| return new UnlimitedTaskContentService(this._fieldConverterService, this._snackBarService, this._translate, this._log, this._injector); | |
| } |
🤖 Prompt for AI Agents
In
projects/netgrif-components-core/src/lib/task-content/services/task-content-service-factory.ts
around lines 21 to 27, add JSDoc comments above each public factory method
describing the purpose and intended usage: document
createSingleTaskContentService as returning a SingleTaskContentService for
single-task contexts (briefly noting expected behavior/constraints) and document
createUnlimitedTaskContentService as returning an UnlimitedTaskContentService
for multi- or unlimited-task contexts (briefly noting differences and when to
prefer it); keep comments concise, mention parameter-less methods and return
type, and follow existing project JSDoc/style conventions.
| const requestBody = injectedTabData.baseFilter.getRequestBody() as TaskSearchRequestBody | ||
| if (requestBody.transitionId === undefined) { |
There was a problem hiding this comment.
Guard against null/undefined requestBody before property access.
The type assertion on line 35 doesn't guarantee that getRequestBody() returns a non-null value. If it returns null or undefined, line 36's property access will throw a TypeError at runtime.
Apply this diff to add a null check:
function baseFilterFactory(injectedTabData: InjectedTabbedTaskViewDataWithNavigationItemTaskData) {
- const requestBody = injectedTabData.baseFilter.getRequestBody() as TaskSearchRequestBody
- if (requestBody.transitionId === undefined) {
+ const requestBody = injectedTabData.baseFilter.getRequestBody() as TaskSearchRequestBody;
+ if (requestBody && requestBody.transitionId === undefined) {
const viewDataGroups: Array<DataGroup> = injectedTabData.navigationItemTaskData?.slice(4, injectedTabData.navigationItemTaskData.length);🤖 Prompt for AI Agents
In
projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/tabbed/default-tabbed-single-task-view/default-tabbed-single-task-view.component.ts
around lines 35 to 36, the code asserts getRequestBody() as
TaskSearchRequestBody but then accesses requestBody.transitionId without
guarding against null/undefined; update the code to first retrieve the result
into a local variable, check if it is null or undefined and handle that case
(return early or set a safe default) before accessing transitionId, ensuring no
property access occurs on a nullish value.
| function baseFilterFactory(injectedTabData: InjectedTabbedTaskViewDataWithNavigationItemTaskData) { | ||
| const requestBody = injectedTabData.baseFilter.getRequestBody() as TaskSearchRequestBody | ||
| if (requestBody.transitionId === undefined) { | ||
| const viewDataGroups: Array<DataGroup> = injectedTabData.navigationItemTaskData?.slice(4, injectedTabData.navigationItemTaskData.length); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Explain or extract the magic number 4 and simplify slice usage.
The hardcoded index 4 lacks context—why skip the first 4 elements? This makes maintenance difficult if the data structure changes.
Additionally, slice(4, array.length) is equivalent to slice(4).
Consider applying this diff:
+ // First 4 data groups contain [specific purpose - add comment explaining why],
+ // remaining groups contain the transition_id field
- const viewDataGroups: Array<DataGroup> = injectedTabData.navigationItemTaskData?.slice(4, injectedTabData.navigationItemTaskData.length);
+ const VIEW_DATA_START_INDEX = 4;
+ const viewDataGroups: Array<DataGroup> = injectedTabData.navigationItemTaskData?.slice(VIEW_DATA_START_INDEX);📝 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.
| const viewDataGroups: Array<DataGroup> = injectedTabData.navigationItemTaskData?.slice(4, injectedTabData.navigationItemTaskData.length); | |
| // First 4 data groups contain [specific purpose – add comment explaining why], | |
| // remaining groups contain the transition_id field | |
| const VIEW_DATA_START_INDEX = 4; | |
| const viewDataGroups: Array<DataGroup> = | |
| injectedTabData.navigationItemTaskData?.slice(VIEW_DATA_START_INDEX); |
🤖 Prompt for AI Agents
In
projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/tabbed/default-tabbed-single-task-view/default-tabbed-single-task-view.component.ts
around line 37, replace the magic number 4 and simplify the slice call:
introduce a clearly named constant (e.g., SKIP_INITIAL_GROUPS or
FIRST_DATA_GROUP_INDEX) or compute the start index with a descriptive helper so
the reason for skipping the first elements is explicit, change slice(4,
injectedTabData.navigationItemTaskData.length) to slice(startIndex) and add a
brief comment explaining why those initial elements are omitted.
| const viewTransitionId = extractFieldValueFromData<string>(viewDataGroups, "transition_id"); | ||
| if (viewTransitionId !== undefined) { | ||
| return { | ||
| filter: injectedTabData.baseFilter.merge(new SimpleFilter('', FilterType.TASK, {transitionId: viewTransitionId?.split(",")}), MergeOperator.AND) | ||
| }; | ||
| } |
There was a problem hiding this comment.
Wrap field extraction in error handling to prevent runtime exceptions.
extractFieldValueFromData throws an error if the field "transition_id" doesn't exist (see relevant snippet). If this field is not guaranteed to be present in all navigation items, the factory will crash at runtime.
Apply this diff to handle missing fields gracefully:
if (viewDataGroups !== undefined) {
- const viewTransitionId = extractFieldValueFromData<string>(viewDataGroups, "transition_id");
- if (viewTransitionId !== undefined) {
+ try {
+ const viewTransitionId = extractFieldValueFromData<string>(viewDataGroups, "transition_id");
+ if (viewTransitionId !== undefined && viewTransitionId !== '') {
+ return {
+ filter: injectedTabData.baseFilter.merge(new SimpleFilter('', FilterType.TASK, {transitionId: viewTransitionId.split(",")}), MergeOperator.AND)
+ };
+ }
+ } catch (error) {
+ // Field not found, fallback to base filter
+ }
+ }
+ }
+ return {
+ filter: injectedTabData.baseFilter
+ };
+}Note: Also removed the redundant optional chaining (viewTransitionId?.split) since we already verify it's defined.
📝 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.
| const viewTransitionId = extractFieldValueFromData<string>(viewDataGroups, "transition_id"); | |
| if (viewTransitionId !== undefined) { | |
| return { | |
| filter: injectedTabData.baseFilter.merge(new SimpleFilter('', FilterType.TASK, {transitionId: viewTransitionId?.split(",")}), MergeOperator.AND) | |
| }; | |
| } | |
| if (viewDataGroups !== undefined) { | |
| try { | |
| const viewTransitionId = extractFieldValueFromData<string>(viewDataGroups, "transition_id"); | |
| if (viewTransitionId !== undefined && viewTransitionId !== '') { | |
| return { | |
| filter: injectedTabData.baseFilter.merge(new SimpleFilter('', FilterType.TASK, {transitionId: viewTransitionId.split(",")}), MergeOperator.AND) | |
| }; | |
| } | |
| } catch (error) { | |
| // Field not found, fallback to base filter | |
| } | |
| } | |
| } | |
| } | |
| return { | |
| filter: injectedTabData.baseFilter | |
| }; | |
| } |
🤖 Prompt for AI Agents
projects/netgrif-components/src/lib/navigation/group-navigation-component-resolver/default-components/tabbed/default-tabbed-single-task-view/default-tabbed-single-task-view.component.ts
lines 39-44: extractFieldValueFromData can throw if "transition_id" is missing,
so wrap the extraction in a try/catch (or check existence first) and only
build/return the filter when extraction succeeds and yields a defined string; on
error or undefined, fall through (return undefined or the existing default) to
avoid crashing, and remove the redundant optional chaining on split since you
will only call split when the value is defined.
| InjectedTabData, SimpleFilter, | ||
| TaskContentServiceFactory, | ||
| NAE_TASK_CONTENT_SERVICE_TYPE, | ||
| TaskContentServiceType |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused import.
The SimpleFilter import is not used anywhere in this file.
Apply this diff to remove the unused import:
- InjectedTabData, SimpleFilter,
+ InjectedTabData,📝 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.
| InjectedTabData, SimpleFilter, | |
| TaskContentServiceFactory, | |
| NAE_TASK_CONTENT_SERVICE_TYPE, | |
| TaskContentServiceType | |
| InjectedTabData, | |
| TaskContentServiceFactory, | |
| NAE_TASK_CONTENT_SERVICE_TYPE, | |
| TaskContentServiceType |
🤖 Prompt for AI Agents
In projects/netgrif-components/src/lib/panel/task-panel/task-panel.component.ts
around lines 31 to 34, the import list includes an unused symbol `SimpleFilter`;
remove `SimpleFilter` from the import clause so the import becomes:
InjectedTabData, TaskContentServiceFactory, NAE_TASK_CONTENT_SERVICE_TYPE,
TaskContentServiceType (adjust commas/spacing as needed) to eliminate the unused
import.
| import {PanelComponentModule} from '../../../panel/panel.module'; | ||
| import {AuthenticationComponentModule} from '../../../authentication/auth.module'; |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Import paths correctly updated.
The import paths have been properly adjusted to reflect the new module locations (panel and authentication moved from lib/view/ to lib/ level).
Verify that all import path references to PanelComponentModule and AuthenticationComponentModule have been consistently updated across the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Verify all imports of PanelComponentModule and AuthenticationComponentModule use the new paths.
# Search for imports of PanelComponentModule
echo "=== Checking PanelComponentModule imports ==="
rg -n --type=ts "from ['\"].*panel\.module['\"]" -A 1 -B 1
echo ""
echo "=== Checking AuthenticationComponentModule imports ==="
rg -n --type=ts "from ['\"].*auth\.module['\"]" -A 1 -B 1Length of output: 17693
PanelComponentModule import paths not updated consistently
Multiple files still import PanelComponentModule from outdated paths (e.g. ../../panel/panel.module, ../panel/panel.module). Update every occurrence to ../../../panel/panel.module to match the new module location.
🤖 Prompt for AI Agents
projects/netgrif-components/src/lib/view/task-view/task-panel-list/task-list.component.spec.ts
lines 28-29: the PanelComponentModule import path should use the updated
relative location; replace any import of PanelComponentModule that uses outdated
paths (e.g. ../../panel/panel.module or ../panel/panel.module) with
../../../panel/panel.module, ensure the import identifier remains unchanged,
update all other files in the repo that reference PanelComponentModule to the
same ../../../panel/panel.module path, then run the TypeScript build/tests to
verify no unresolved import errors remain.
machacjozef
changed the base branch from
release/7.0.0-rev9
to
release/7.0.0-rev8
| if (this._taskContentService instanceof UnlimitedTaskContentService && this.panelRef) { | ||
| this.collapse(); | ||
| this._taskContentService.task = this._taskPanelData.task; | ||
| if (this._sub) { |
There was a problem hiding this comment.
I know it is not the code of the author, and this is only for future reference: attribute name _sub does not express what is the purpose of this attribute. I searched up, that this attribute is a subscription of changedFields, so something like _changedFieldSub would me better.
|
Description
Fixes NAE-2217
Dependencies
Third party dependencies
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
manually
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc. You can use >
Checklist:
Summary by CodeRabbit
New Features
Refactor