Skip to content

Conversation

@Retoocs
Copy link
Contributor

@Retoocs Retoocs commented Oct 14, 2025

Description

The problem was, that if you finish the task, which updates any taskRef, that is referenced in the task you are finishing, frontend tries to do reload. I have updated the implementation, that backend sends information, that the task which is being finished, does not exist and the frontend will not reload the task itself.

Fixes NAE-2231

Dependencies

No new dependencies were introduced

Third party dependencies

No new dependencies were introduced

Blocking Pull requests

There are no dependencies on other PR

How Has Been This Tested?

Manually

Test Configuration

Name Tested on
OS Ubuntu 24.04.1 LTS
Runtime Node 23.6.1
Dependency Manager NPM 11.0.0
Framework version Angular 19.2.2
Run parameters
Other configuration

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes have been checked, personally or remotely, with @Kovy95
  • I have commented my code, particularly in hard-to-understand areas
  • I have resolved all conflicts with the target branch of the PR
  • I have updated and synced my code with the target branch
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes:
    • Lint test
    • Unit tests
    • Integration tests
  • I have checked my contribution with code analysis tools:
  • I have made corresponding changes to the documentation:
    • Developer documentation
    • User Guides
    • Migration Guides

Summary by CodeRabbit

  • New Features

    • Tasks now expose whether they are still executable.
    • Finishing a task updates and propagates the task’s executability status so UI and services reflect current state.
  • Bug Fixes

    • Task content reloads after reference changes only when the task is still executable, reducing unnecessary updates and preventing unintended refreshes.

✏️ Tip: You can customize this high-level summary in your review settings.

…ut error message

- fix reloading a non-existing task
@coderabbitai
Copy link

coderabbitai bot commented Oct 14, 2025

Walkthrough

Adds optional executable-state flags to the FinishTaskEventOutcome and Task interfaces; finish flow now forwards that flag into task-state updates. TaskContentService stores the flag and uses it to gate TASK_REF-triggered task data reloads.

Changes

Cohort / File(s) Summary of changes
Interfaces: Executable state flags
projects/netgrif-components-core/src/lib/event/model/event-outcomes/task-outcomes/finish-task-event-outcome.ts, projects/netgrif-components-core/src/lib/resources/interface/task.ts
Added optional booleans: FinishTaskEventOutcome.isTaskStillExecutable? and Task.isStillExecutable?.
Services: Propagation and gating
projects/netgrif-components-core/src/lib/task-content/services/task-content.service.ts, projects/netgrif-components-core/src/lib/task/services/finish-task.service.ts
finish handling now passes isTaskStillExecutable into TaskContentService.updateStateData(outcome, flag). TaskContentService stores it on this._task.isStillExecutable and requires it (true or undefined) to allow TASK_REF-triggered reloads.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant UI as User / UI
  participant F as FinishTaskService
  participant B as Backend
  participant T as TaskContentService

  UI->>F: trigger finish task
  F->>B: POST /tasks/{id}/finish
  B-->>F: outcomeResource { outcome, success, isTaskStillExecutable }
  alt success
    F->>T: updateStateData(outcome, outcome.isTaskStillExecutable)
    T->>T: this._task.isStillExecutable = outcome.isTaskStillExecutable
  else failure
    F-->>UI: emit error/result
  end
Loading
sequenceDiagram
  participant UI as UI (field update)
  participant T as TaskContentService

  UI->>T: updateField(field)
  alt field.type == TASK_REF and (this._task.isStillExecutable === true or undefined)
    T->>T: trigger task data reload
  else
    T->>T: skip reload (task not executable)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review updateStateData signature and callers to ensure default and propagation are correct.
  • Confirm serialization/deserialization of isStillExecutable across API responses.
  • Inspect TASK_REF gating logic to verify no unintended reload suppression.

Possibly related PRs

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title references the bug fix (NAE-2231) and summarizes the core issue: preventing taskRef reload behavior on finish events, which aligns with the changeset's additions of isStillExecutable and isTaskStillExecutable properties.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a7eb4fd and a7853f0.

📒 Files selected for processing (1)
  • projects/netgrif-components-core/src/lib/task-content/services/task-content.service.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-18T15:00:21.234Z
Learnt from: machacjozef
Repo: netgrif/components PR: 281
File: projects/netgrif-components-core/src/lib/navigation/navigation-double-drawer/abstract-navigation-double-drawer.ts:0-0
Timestamp: 2025-07-18T15:00:21.234Z
Learning: The user machacjozef prefers to keep double-negation (!!) in conditional statements for safety and clarity, even when the static analysis tool suggests removing it. They believe it helps prevent subtle bugs with truthy/falsy values and makes the boolean conversion explicit.

Applied to files:

  • projects/netgrif-components-core/src/lib/task-content/services/task-content.service.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). (4)
  • GitHub Check: Matrix Test (16)
  • GitHub Check: Matrix Test (14)
  • GitHub Check: Test with SonarCloud
  • GitHub Check: Matrix Test (18)
🔇 Additional comments (3)
projects/netgrif-components-core/src/lib/task-content/services/task-content.service.ts (3)

209-209: LGTM! Backward-compatible signature change.

The addition of the optional isStillExecutable parameter with a default value of true maintains backward compatibility while enabling the new finish-task behavior.


214-214: LGTM! Stores the executable state correctly.

The assignment properly stores the isStillExecutable flag on the task object for use in the TASK_REF reload logic.


239-239: Conditional logic implements the fix correctly.

The condition properly prevents task data reloads for TASK_REF fields when the task is no longer executable (isStillExecutable === false), while maintaining backward compatibility when the flag is undefined.

Note: A past review suggested simplifying this to this._task?.isStillExecutable !== false, which is logically equivalent. However, based on learnings, the double-negation pattern is preferred in this codebase for explicit boolean conversion and clarity.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f307847 and a7eb4fd.

📒 Files selected for processing (4)
  • projects/netgrif-components-core/src/lib/event/model/event-outcomes/task-outcomes/finish-task-event-outcome.ts (1 hunks)
  • projects/netgrif-components-core/src/lib/resources/interface/task.ts (1 hunks)
  • projects/netgrif-components-core/src/lib/task-content/services/task-content.service.ts (2 hunks)
  • projects/netgrif-components-core/src/lib/task/services/finish-task.service.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
projects/netgrif-components-core/src/lib/task/services/finish-task.service.ts (1)
projects/netgrif-components-core/src/lib/event/model/event-outcomes/task-outcomes/finish-task-event-outcome.ts (1)
  • FinishTaskEventOutcome (4-6)
⏰ 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). (4)
  • GitHub Check: Matrix Test (18)
  • GitHub Check: Matrix Test (14)
  • GitHub Check: Matrix Test (16)
  • GitHub Check: Test with SonarCloud
🔇 Additional comments (3)
projects/netgrif-components-core/src/lib/event/model/event-outcomes/task-outcomes/finish-task-event-outcome.ts (1)

5-5: LGTM!

The property addition aligns with the PR objective to track task executability through event outcomes.

projects/netgrif-components-core/src/lib/task-content/services/task-content.service.ts (1)

209-216: LGTM! Default value ensures backward compatibility.

The isStillExecutable parameter defaults to true, which means existing code paths that don't explicitly pass this flag will assume tasks remain executable. This is appropriate for backward compatibility.

projects/netgrif-components-core/src/lib/task/services/finish-task.service.ts (1)

136-137: LGTM! Correctly propagates executable state.

The changes properly extract the isTaskStillExecutable flag from the finish response and pass it to updateStateData, completing the flow to prevent reloading finished tasks.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
38.9% Coverage on New Code (required ≥ 50%)

See analysis details on SonarQube Cloud

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2025

@tuplle tuplle merged commit 3e9c486 into release/6.4.2 Dec 1, 2025
10 of 11 checks passed
@tuplle tuplle deleted the NAE-2231 branch December 1, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants