[ores.qt,ores.refdata.service] Verify party_status Qt UI post-NATS

[mcraveiro]

Summary

Verified party_status Qt UI end-to-end after the NATS migration (list, detail round-trip, history, delete, cross-session eventing). Fixed two regressions found along the way.

Changes

• History window close button now wired to connect_dialog_close
• party_status NATS event mapping/subscription added to ores.refdata.service so changes propagate across sessions

Traceability

Artefact	Link	ID
Story	Commission: party_status	8B3D478B-DD68-44B7-A8BF-DB50F76C9506
Task	Verify party_status Qt UI end-to-end post-NATS	2F91918C-F9BC-4648-9EC2-085EB4D96AD7
Environment	jolly_knuth

🤖 Generated with Claude Code

[mcraveiro]

@claude please review this pull request

[claude]

Claude finished @mcraveiro's task in 1m 49s —— View job

---

Claude finished reviewing this PR

• Gather context (PR description, changed files, diff)
• Understand the request (code review requested via /code-review)
• Review PartyStatusController.cpp change
• Review application.cpp NATS event mapping change
• Review agile/doc changes for consistency
• Post final review feedback

Summary

This PR verifies the party_status Qt UI post-NATS-migration and fixes two small regressions found during that verification. Both fixes are minimal and match established patterns elsewhere in the codebase.

projects/ores.qt/party/src/PartyStatusController.cpp (+2)

• Adds connect_dialog_close(historyDialog, historyWindow); before window tracking (line 322). Matches the exact call pattern (dialog, window) used in every other detail/history controller (e.g. MarketDataGenerationConfigController.cpp, GmmComponentController.cpp). Correctly placed after all dialog signal connections and before track_window/register_detachable_window. No issues.

projects/ores.refdata/service/src/app/application.cpp (+14)

• Adds register_mapping<rdev::party_status_changed_event>(event_source, "ores.refdata.party_status", "ores_refdata_party_statuses"). Verified the channel name ores_refdata_party_statuses matches the pg_notify channel emitted by ores_refdata_party_statuses_notify_trg in refdata_party_statuses_notify_trigger_create.sql, and the entity name "ores.refdata.party_status" matches the trigger's entity_name. Correct.
• Adds the party_status_sub subscription that republishes to NATS via publish_entity_event, using e.codes (matches the party_status_changed_event struct's std::vector<std::string> codes field, and follows the same .codes-based pattern as the neighboring party_type_sub/purpose_type_sub, as opposed to the .ids-based pattern used by party_sub/party_id_sub). Alphabetical placement among the other subscriptions is also consistent. No issues.

Docs (doc/agile/..., doc/llm/skills/devops-run-environment/..., doc/recipes/ops/...)

• Task/story updated to DONE with acceptance checklist ticked and results recorded — consistent with the repo's agile doc conventions.
• New devops-run-environment skill and how_do_i_ready_up_an_environment.org recipe are well-formed org-mode skill docs, properly cross-linked and indexed in s1_agent.org, claude_code_skills.org, and ops.org.

No bugs, security issues, or style concerns found. The changes are small, targeted, and consistent with existing conventions in the codebase.

[claude]

Claude finished @mcraveiro's task in 1m 31s —— View job

---

Code Review

Todo list:

• Gather context (PR diff, changed files)
• Review PartyStatusController.cpp change
• Review application.cpp NATS event mapping change
• Review doc/agile and doc/llm/skills changes
• Post review feedback

---

Summary

Two small, well-scoped regression fixes plus supporting docs. Both changes follow the codebase's existing conventions closely, and I didn't find any correctness issues.

projects/ores.qt/party/src/PartyStatusController.cpp (+2)

connect_dialog_close(historyDialog, historyWindow); added at PartyStatusController.cpp:322, right before the window is tracked/registered. This matches the exact pattern used for every other detail/history window in this codebase (BusinessUnitTypeController.cpp, JobDefinitionController.cpp, and the cpp_qt_controller.cpp.mustache codegen template all call connect_dialog_close at the equivalent point). Correct fix for a missing wiring regression — no notes.

projects/ores.refdata/service/src/app/application.cpp (+14)

Adds the party_status_changed_event mapping registration (application.cpp:157-158) and the NATS publish subscription (application.cpp:329-338). Verified:

• The #include for party_status_changed_event.hpp was already needed and is added at the correct alphabetically-sorted spot.
• The entity name string "ores.refdata.party_status" and event trait name ores::eventing::domain::event_traits<party_status_changed_event>::name ("ores.refdata.party_status_changed") match what PartyStatusController.cpp already subscribes to client-side (PartyStatusController.cpp:40), so this closes the loop correctly — this was indeed the missing piece for cross-session propagation.
• party_status_sub follows the same naming/lifetime convention as its neighbors (party_type_sub, purpose_type_sub, etc.) — held as a local in the same scope for the duration of run(), consistent with the rest of the file.
• Alphabetical placement of both the mapping registration and the subscription block is correct (between party_identifier/party_id_sub and party_type/party_type_sub).

No issues found.

Docs (story.org, task_verify_qt_party_status.org, skill/recipe additions)

Traceability, acceptance checklist, and Result section are consistent with the PR description and each other. Nothing to flag — this is process documentation, not behavior-affecting code.

Minor (non-blocking) nit

In task_verify_qt_party_status.org, the | State | DONE | and | Now | Nothing. | table cells have inconsistent trailing padding compared to the rest of the table (org-mode will auto-realign on next edit, so purely cosmetic).

---

No blocking issues. This looks good to merge.

[claude]

Claude finished @mcraveiro's task in 2m 19s —— View job

---

Claude finished reviewing this PR

Tasks:

• Gather context (PR diff, changed files)
• Understand the request
• Review code changes
• Post review feedback

---

Summary

The two functional fixes are correct and follow existing conventions closely:

• PartyStatusController.cpp:322 — connect_dialog_close(historyDialog, historyWindow); matches the exact pattern used for detail windows elsewhere in the same file and in sibling controllers.
• application.cpp — the party_status_changed_event mapping (ores.refdata.party_status → ores_refdata_party_statuses) and the party_status_sub subscription are wired up identically to every other entity in this file (table name confirmed against refdata_party_statuses_create.sql), and the event struct (timestamp/codes/tenant_id) matches the shape publish_entity_event expects.

The doc/agile updates (task closure, story status, notes, PR link) are consistent with the code changes, and the new devops-run-environment skill/recipe additions are simple index/doc additions with no issues.

Finding: the history-close-button bug is not unique to party_status

I checked whether the connect_dialog_close fix applied here (PartyStatusController.cpp:322) is also missing in the other party controllers' showHistoryWindow() methods — since they share the same structure (created via the same code-gen pattern per doc/functions). It is: 7 other controllers have the identical gap, so their history windows likely have the same non-functional close button:

• BusinessCentreController::showHistoryWindow (BusinessCentreController.cpp:268)
• BusinessUnitController::showHistoryWindow (BusinessUnitController.cpp:276)
• BusinessUnitTypeController::showHistoryWindow (BusinessUnitTypeController.cpp:261)
• ContactTypeController::showHistoryWindow (ContactTypeController.cpp:261)
• CounterpartyController::showHistoryWindow (CounterpartyController.cpp:318)
• PartyController::showHistoryWindow (PartyController.cpp:294)
• PartyIdSchemeController::showHistoryWindow (PartyIdSchemeController.cpp:261)

All of these build historyDialog/historyWindow and call track_window/register_detachable_window/show_managed_window, but never call connect_dialog_close, unlike their own showDetailWindow counterparts (which do call it consistently). This is out of scope for a "verify party_status" task, but since the fix here is a copy-paste pattern that's likely shared across the code-gen template, it's worth a follow-up capture/task to apply the same one-line fix to the other 7 controllers so their history windows' close buttons work too.

Fix history window close buttons →

Minor observation (informational, not blocking)

While checking event registration, I noticed several other *_changed_event types under ores.refdata.api/eventing/ (e.g. contact_type_changed_event, party_id_scheme_changed_event, book_status_changed_event, and various convention types) aren't registered/subscribed in application.cpp either. This is pre-existing and unrelated to this PR's scope, but contact_type and party_id_scheme do have Qt controllers with history windows, so they may have the same "changes don't propagate across sessions" gap that was just fixed for party_status. Worth a backlog capture if not already tracked.

No blocking issues found for this PR as scoped.
· Branch: feature/verify-qt-party-status

[mcraveiro]

Thanks for the review — no changes requested, confirming out of the review round.

[mcraveiro]

Thanks — the org-mode table padding nit is cosmetic and will auto-realign on next edit, leaving as-is.

[mcraveiro]

Good catch on the other 7 controllers with the same missing connect_dialog_close wiring, and the unregistered *_changed_event types — both out of scope for this verification task, filing backlog captures for both so they get picked up separately.

[claude]

Claude finished @mcraveiro's task in 1m 38s —— View job

---

Claude finished reviewing this PR

• Gather context (PR diff, prior review rounds, comments)
• Confirm whether anything changed since the last review round
• Re-review changed files
• Post final review feedback

---

Summary

Re-checked the PR at the current HEAD (9f78c4af) — no new commits have landed since the last review round, so the diff is unchanged from what was already reviewed:

• projects/ores.qt/party/src/PartyStatusController.cpp:322 — connect_dialog_close(historyDialog, historyWindow); is correctly placed after all dialog signal connections and before track_window/register_detachable_window, matching the pattern used by every other detail/history controller. Confirmed.
• projects/ores.refdata/service/src/app/application.cpp:157-158, 329-338 — party_status_changed_event mapping ("ores.refdata.party_status" → "ores_refdata_party_statuses") and the party_status_sub subscription (using e.codes, consistent with the neighboring .codes-based subscriptions) are wired identically to the rest of the file, and alphabetically placed correctly. Confirmed.
• Doc/agile changes remain consistent with the code changes and with each other.

No issues found — this confirms the three prior review passes. No blocking issues for this PR as scoped.

Two follow-ups were already identified in the previous round and acknowledged by @mcraveiro as backlog items (not part of this PR's scope):

1. The same missing connect_dialog_close wiring in 7 other controllers' showHistoryWindow() methods.
2. Several other *_changed_event types not yet registered/subscribed in application.cpp (e.g. contact_type_changed_event, party_id_scheme_changed_event).

This looks good to merge.

[claude]

Claude finished @mcraveiro's task in 59s —— View job

---

Claude finished reviewing this PR

Tasks:

• Gather context (PR diff, changed files)
• Check for new commits since the last review round
• Review the two new backlog capture docs
• Post review feedback

---

Summary

The latest push (f6b11fd2) only adds documentation — no functional code changed since the prior review rounds on PartyStatusController.cpp and application.cpp, which were already reviewed and found correct (no issues). The new commit:

• Records the review round on task_verify_qt_party_status.org
• Adds two product-backlog captures for the follow-up items identified in the previous review pass

doc/agile/product_backlog/inbox/fix_history_close_button_other_controllers.org

Accurately captures the finding: 7 controllers (BusinessCentreController, BusinessUnitController, BusinessUnitTypeController, ContactTypeController, CounterpartyController, PartyController, PartyIdSchemeController) are missing the same connect_dialog_close wiring in showHistoryWindow() that was just fixed for party_status. Correctly notes the likely root cause (codegen template) worth checking. Well-formed, properly linked to the inbox capture bucket and PR #1462.

doc/agile/product_backlog/inbox/register_missing_refdata_nats_events.org

Accurately captures the finding that several *_changed_event types (contact_type_changed_event, party_id_scheme_changed_event, book_status_changed_event, etc.) aren't registered in application.cpp, and correctly flags contact_type/party_id_scheme as the highest-risk cases since they have Qt history windows. Well-formed.

Both captures are properly scoped as backlog items rather than folded into this PR, which is the right call — they're out of scope for a "verify party_status" task.

No blocking issues. This PR remains good to merge; the two follow-ups are correctly tracked separately.
· Branch: feature/verify-qt-party-status

[claude]

Claude finished @mcraveiro's task in 1m 27s —— View job

---

Code Review

Todo list:

• Gather context (PR diff, changed files)
• Review new commits since last review round (agile doc bookkeeping)
• Post review feedback

---

Summary

This sync only adds documentation — no functional code changed since the last review round (PartyStatusController.cpp and application.cpp are unchanged from what was already reviewed three times with no issues). The new commits record the outcome of that review round:

• doc/agile/product_backlog/inbox/fix_history_close_button_other_controllers.org (new) — capture for the 7-controller connect_dialog_close gap found during review. Correctly cross-references PR [ores.qt,ores.refdata.service] Verify party_status Qt UI post-NATS #1462 and lists all 7 affected controllers.
• doc/agile/product_backlog/inbox/register_missing_refdata_nats_events.org (new) — capture for the other unregistered *_changed_event types found during review. Correctly scoped and references PR [ores.qt,ores.refdata.service] Verify party_status Qt UI post-NATS #1462.
• doc/agile/versions/v0/sprint_22/commission_party_status/task_verify_qt_party_status.org — adds a * Review table recording all 4 review points (LGTM x3, the cosmetic table-padding nit declined, and the two accepted follow-ups with correct capture IDs 8E266A23-F282-4142-93B5-CECEEA3FE380 and 6908F023-0F48-4BB7-B0D8-E05CE63E5033 — both match the IDs in the two new capture files above).
• story.org — minor status bookkeeping update, consistent with the task change.

Everything here is internally consistent: the capture files' PR references, the task's review table, and the actual review comments in this thread all line up correctly. No issues found.

No blocking issues. This looks good to merge.