-
Notifications
You must be signed in to change notification settings - Fork 4
[NAE-2217] Single Task View #303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a846664
c545b10
862073e
8309d60
c79cdc6
6ee6330
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,7 +15,7 @@ import {MatExpansionPanel} from '@angular/material/expansion'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {ComponentPortal} from '@angular/cdk/portal'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {TaskContentService} from '../../task-content/services/task-content.service'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {LoggerService} from '../../logger/services/logger.service'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {TaskPanelData} from '../task-panel-list/task-panel-data/task-panel-data'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {TaskPanelData} from '../task-panel-data/task-panel-data'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {Observable, Subscription} from 'rxjs'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {TaskViewService} from '../../view/task-view/service/task-view.service'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {filter, map, take} from 'rxjs/operators'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -53,6 +53,7 @@ import { FinishPolicyService } from '../../task/services/finish-policy.service'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {NAE_TAB_DATA} from '../../tabs/tab-data-injection-token/tab-data-injection-token'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {InjectedTabData} from '../../tabs/interfaces'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {AfterAction} from '../../utility/call-chain/after-action'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {UnlimitedTaskContentService} from "../../task-content/services/unlimited-task-content.service"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Component({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| selector: 'ncc-abstract-legal-notice', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -257,6 +258,17 @@ export abstract class AbstractTaskPanelComponent extends AbstractPanelWithImmedi | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Input() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public set taskPanelData(data: TaskPanelData) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this._taskPanelData = data; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+261
to
+271
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 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:
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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.resolveFeaturedFieldsValues(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| import {InjectionToken} from '@angular/core'; | ||
| import {TaskContentServiceType} from './task-content-service-type'; | ||
|
|
||
| export const NAE_TASK_CONTENT_SERVICE_TYPE = new InjectionToken<TaskContentServiceType>('NaeTaskContentServiceType'); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| export enum TaskContentServiceType { | ||
| /** | ||
| * Type for creating SingleTaskContentService | ||
| */ | ||
| SINGLE = 'single', | ||
| /** | ||
| * Type for creating UnlimitedTaskContentService | ||
| */ | ||
| UNLIMITED = 'unlimited' | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,28 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import {Injectable, Injector} from "@angular/core"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import {LoggerService} from "../../logger/services/logger.service"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import {FieldConverterService} from "./field-converter.service"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import {SnackBarService} from "../../snack-bar/services/snack-bar.service"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import {TranslateService} from "@ngx-translate/core"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import {SingleTaskContentService} from "./single-task-content.service"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import {UnlimitedTaskContentService} from "./unlimited-task-content.service"; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @Injectable({ | ||||||||||||||||||||||||||||||||||||||||||||||
| providedIn: 'root' | ||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||
| export class TaskContentServiceFactory { | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| constructor(protected _fieldConverterService: FieldConverterService, | ||||||||||||||||||||||||||||||||||||||||||||||
| protected _snackBarService: SnackBarService, | ||||||||||||||||||||||||||||||||||||||||||||||
| protected _translate: TranslateService, | ||||||||||||||||||||||||||||||||||||||||||||||
| protected _log: LoggerService, | ||||||||||||||||||||||||||||||||||||||||||||||
| protected _injector: Injector) { | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| 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); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,13 +19,31 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||
| TaskRequestStateService, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| TaskViewService, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ViewIdService, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| TaskSearchRequestBody, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| DataGroup, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| extractFieldValueFromData, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| SimpleFilter, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| FilterType, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| MergeOperator | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } from '@netgrif/components-core'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import {AsyncPipe} from "@angular/common"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| InjectedTabbedTaskViewDataWithNavigationItemTaskData | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "../../model/injected-tabbed-task-view-data-with-navigation-item-task-data"; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| function baseFilterFactory(injectedTabData: InjectedTabbedTaskViewDataWithNavigationItemTaskData) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const requestBody = injectedTabData.baseFilter.getRequestBody() as TaskSearchRequestBody | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (requestBody.transitionId === undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+35
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard against null/undefined requestBody before property access. The type assertion on line 35 doesn't guarantee that 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 |
||||||||||||||||||||||||||||||||||||||||||||||||||
| const viewDataGroups: Array<DataGroup> = injectedTabData.navigationItemTaskData?.slice(4, injectedTabData.navigationItemTaskData.length); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Explain or extract the magic number 4 and simplify slice usage. The hardcoded index Additionally, 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| if (viewDataGroups !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const viewTransitionId = extractFieldValueFromData<string>(viewDataGroups, "transition_id"); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (viewTransitionId !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| filter: injectedTabData.baseFilter.merge(new SimpleFilter('', FilterType.TASK, {transitionId: viewTransitionId?.split(",")}), MergeOperator.AND) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+39
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrap field extraction in error handling to prevent runtime exceptions.
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 ( 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| filter: injectedTabData.baseFilter | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it is not the code of the author, and this is only for future reference: attribute name
_subdoes not express what is the purpose of this attribute. I searched up, that this attribute is a subscription ofchangedFields, so something like_changedFieldSubwould me better.