Skip to content

feat: show state and responsible subject changes in issue activity - #70

Open
xhyrom wants to merge 78 commits into
mainfrom
feat/new-activity-types
Open

xhyrom wants to merge 78 commits into
mainfrom
feat/new-activity-types

Conversation

@xhyrom

@xhyrom xhyrom commented Apr 17, 2026 •

Copy link
Copy Markdown
Member

The problem

Closes #36
Refs https://github.com/slovensko-digital/ops/issues/1181

When an operator changes an issue's state or its responsible subject (RS) in Triage, nothing shows up on the portal unless they also write a comment. Often the RS is quietly switched to "Iný subjekt", and to the author and subscribers the issue looks abandoned. The change history is visible in Triage, but not to the people who reported or follow the issue. Subscribers also get no email when an issue goes in progress or moves to a different RS.

Solution

State changes and RS changes are now recorded as activities in the issue's timeline, next to comments and updates. The section heading changes from Diskusia to Aktivita.

  • Recording. Two new activity types, Issues::StateChangeActivity → Issues::StateChange and Issues::SystemResponsibleSubjectChangeActivity → Issues::SystemResponsibleSubjectChange, are created by after_update callbacks in app/models/issue.rb whenever state_id or responsible_subject_id changes, no matter what triggered the change. Both return triage_visible? == false, so they are never synced back to Triage (Triage already keeps its own history).
  • Rendering. issues/state_changes/_state_change and issues/system_responsible_subject_changes/_system_responsible_subject_change show a compact "previous → new" row with state badges. A missing previous value appears as neznámy stav / neznámy.
  • Notifications. A new issue_in_progress email goes out from PublishIssueStateChangedJob. A new PublishIssueResponsibleSubjectChangedJob sends issue_responsible_subject_changed to active, email-notifiable subscribers. The RS email is only sent when the state did not change in the same update, because the state-change notification takes priority.
  • Backfill. Triage::BackfillIssueHistoryJob rebuilds both histories for existing issues from Zammad's ticket_history endpoint (new ZammadApiClient#get_ticket_history). It reads both the triage and resolution tickets, maps ops_state and responsible_subject values to portal records, and inserts the entries in bulk with their original timestamps. It does nothing for an issue that already has activities of a given type. When the latest Zammad value differs from the current portal value, it logs this and imports the Zammad history as is.

Deliberately out of scope: other edits to an issue (title, description, category) are not tracked yet.

Verify: run bin/rails test test/lib/zammad_api_client_test.rb test/jobs/notifications test/jobs/triage/backfill_issue_history_job_test.rb, then change an issue's state and RS (e.g. in the console) and open its detail page.

Demo

image

Checklist

  • docs/how-it-works/ is updated, or this PR changes no flow (job, webhook, Triage or Backoffice trigger, issue state, email, scheduled job)

xhyrom and others added 30 commits April 17, 2026 21:55
# Conflicts:
#	db/structure.sql
The fulltext_search method was vulnerable to SQL injection via apostrophes
and other special characters in user input that break PostgreSQL tsquery
syntax (e.g., ' dunaji'123 ' causes PG::SyntaxError).

Changes:
- Add DISALLOWED_TSQUERY_CHARACTERS regex to filter out dangerous chars
- Extract sanitize_terms method for better testability and readability
- Sanitize query by replacing ' ? \ : with spaces before splitting
- Return empty scope (none) if all terms are sanitized away
- Add comprehensive tests for both fulltext_search and sanitize_terms
- Add Brakeman ignore for false positive SQL injection warning
  (against/unaccent_f are controlled by developer, not user input)

Fixes: ERROR: syntax error in tsquery: "' dunaji'123 '"
Make jobs communicating with Zammad idempotent, able to recover from previous uncompleted runs without failing again

closes #42
Otherwise it crashes if env variables are not set
If somebody starts their local .env from .env.sample, they need this env variable to be `present?`. If it's blank here, local mail delivery does not work.
@xhyrom
xhyrom requested a review from jsuchal October 1, 2026 19:51
@xhyrom

xhyrom commented Oct 1, 2026 •

Copy link
Copy Markdown
Member Author
image

Prípadne ešte môžeme pri RS change spraviť špeciálny prípad kedy sa priradí RS, resp. keď nikto ešte nebol priradený.

@jsuchal

jsuchal commented Oct 2, 2026

Copy link
Copy Markdown
Member

@xhyrom tu este mozno otazka, ze ci je nejako aspon ciastocne mozne spatne zrekonstruovat historiu pre podnety?

@jsuchal

jsuchal commented Oct 2, 2026

Copy link
Copy Markdown
Member
image

Toto je dobry screen, ale nadpis "Diskusia" uz asi nedava uplne zmysel.

@xhyrom

xhyrom commented Oct 2, 2026

Copy link
Copy Markdown
Member Author

@xhyrom tu este mozno otazka, ze ci je nejako aspon ciastocne mozne spatne zrekonstruovat historiu pre podnety?

áno, vedeli by sme to všetko spätne získať z triáže, keďže zammad ukladá históriu všetkých zmien fieldov.

Comment thread db/migrate/20261011120000_backfill_issue_state_changes_from_triage_history.rb Outdated
@mirrec

mirrec commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

@xhyrom v main je uz lepsie otestovany Zammad client, ktory namiesto mockovania ZammadAPI::Client podvrhne fakove HTTP requesty. Treba potom podla toho upravit aj zmeny tuto. O tom ako testovat je aj zmienka v docs/TESTING.md

Comment thread db/migrate/20261011120000_backfill_issue_state_changes_from_triage_history.rb Outdated
@xhyrom
xhyrom requested a review from jsuchal October 17, 2026 14:45
@mirrec mirrec changed the title feat: new activity types feat: show state and responsible subject changes in issue activity Oct 19, 2026
PublishIssueResponsibleSubjectChangedJob called ResponsibleSubject.find(nil)
when an issue got its first responsible subject (nil -> X), e.g. from the
Triage 700 trigger, and failed. A nil previous subject is now handled
explicitly, an unknown id still fails the job.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zmena RS alebo stavu ako zaznam aktivity na portali + notifikacie

7 participants