Sep 4 - Add link relavant issues function

This commit is contained in:
2026-09-04 13:17:33 -04:00
parent b7bc0f0335
commit 50df63115e
7 changed files with 1320 additions and 15 deletions
+42 -2
View File
@@ -359,6 +359,28 @@ notifications: id, user_id, title, body, link, is_read, created_at, issue_id,
notification_preferences: id, user_id, event_type, email_enabled, digest_mode, digest_frequency notification_preferences: id, user_id, event_type, email_enabled, digest_mode, digest_frequency
``` ```
### IssueLink (Phase 54)
```
issue_links: id, issue_id (FK→issues CASCADE, indexed),
linked_issue_id (FK→issues CASCADE, indexed),
link_type ENUM('duplicate','related') NOT NULL DEFAULT 'related',
created_by (FK→users SET NULL), created_at
UniqueConstraint(issue_id, linked_issue_id) — uq_issue_link
```
**Connects a duplicate to its original, or two issues about the same thing**, so whoever picks one up can reach the other.
**One row is stored per pair and shown on BOTH issues.** The stored direction carries meaning for `duplicate``issue_id` is a duplicate *of* `linked_issue_id` — so the same row reads differently at each end: "Duplicate of #B" on one, "Duplicated by #A" on the other. `related` is symmetric and reads "Related to" from either side. `IssueLink.LABELS` is keyed `(link_type, is_source)` and is the only place that wording lives; `label_for(viewing_issue_id)` / `other_issue(viewing_issue_id)` resolve a row against whichever issue is on screen.
Storing one row rather than a mirrored pair keeps the direction unambiguous and makes unlinking a single delete. The cost: **uniqueness cannot be expressed by the UniqueConstraint alone.** `(A,B)` and `(B,A)` are distinct rows to MySQL but the same link to a person, so **`IssueLink.exists_between(a, b)` is the only correct duplicate check** — it looks both ways. The constraint catches the exact-duplicate row; `exists_between()` catches the reverse.
**A link is PURELY NAVIGATIONAL** (decided Sep 2026). Marking a duplicate does **not** touch either issue's status, `resolved_at`, SLA, assignee or followers, and fires no notification. Closing the duplicate stays a separate, deliberate action. Do not add side effects here without saying so in the UI — the link control reads as navigation, and a status write from it would be invisible.
**Two FKs from one table to `issues`, so both relationships pin `foreign_keys`** — rule 86's failure mode, which raises on first ORM *use*, not at import. `Issue.links_from` / `Issue.links_to` are the two storage directions; **`Issue.all_links()` merges them** into the single list a person actually sees. Both relationships cascade `all, delete-orphan` (and both FKs are `ON DELETE CASCADE`), so deleting an issue takes its links from *either* end — a surviving link would render a dead row on the other issue's page.
**Scope is the thing to get right here — see rule 99.**
### IssueComment ### IssueComment
``` ```
@@ -621,7 +643,7 @@ Management of the underlying routes is otherwise unchanged; **Start** is the **a
| `customers` | `/customers` | **Owns BOTH customer roles (Phase 51).** `GET /` list (both roles, role badge + per-role scope column), `GET/POST /new` invite (role select: Customer Director / Customer Inspector — same invitation flow for both), `/set-password/<token>`, `GET /<id>` manage, `/<id>/edit`, `POST /<id>/assignments/add` + `/assignments/<aid>/remove` (**director only** — `CustomerAssignment`), `POST /<id>/contracts` (**inspector only** — replaces the whole `InspectorAssignment` set, rule 59 semantics), `POST /<id>/notifications` (per-account matrix overrides), `POST /<id>/switch-role` (**admin only** — mirrors contracts across, revokes tokens/devices), `POST /<id>/toggle-active`, `POST /<id>/resend-invite`, import CSV | | `customers` | `/customers` | **Owns BOTH customer roles (Phase 51).** `GET /` list (both roles, role badge + per-role scope column), `GET/POST /new` invite (role select: Customer Director / Customer Inspector — same invitation flow for both), `/set-password/<token>`, `GET /<id>` manage, `/<id>/edit`, `POST /<id>/assignments/add` + `/assignments/<aid>/remove` (**director only** — `CustomerAssignment`), `POST /<id>/contracts` (**inspector only** — replaces the whole `InspectorAssignment` set, rule 59 semantics), `POST /<id>/notifications` (per-account matrix overrides), `POST /<id>/switch-role` (**admin only** — mirrors contracts across, revokes tokens/devices), `POST /<id>/toggle-active`, `POST /<id>/resend-invite`, import CSV |
| `inspections` | `/inspections` | list, start, execute, view, PDF export, flag-issue, save-draft (AJAX), flag-followup, reinspect, upload-photo (AJAX), **`POST /bulk`** (bulk export-PDF / request-follow-up / clear-follow-up / delete from the list) | | `inspections` | `/inspections` | list, start, execute, view, PDF export, flag-issue, save-draft (AJAX), flag-followup, reinspect, upload-photo (AJAX), **`POST /bulk`** (bulk export-PDF / request-follow-up / clear-follow-up / delete from the list) |
| `templates` | `/templates` | list, create, edit, delete, form editor, preview | | `templates` | `/templates` | list, create, edit, delete, form editor, preview |
| `issues` | `/issues` | list, view, create, update, verify, comment, follow/unfollow, verification queue, bulk-verify, delete, quick-assign, **`POST /bulk`** (bulk assign / status / verify / delete from the list). **verify / bulk-verify / verification-queue are `@issue_manager_required` (admin/director/auditor); delete stays `@supervisor_required` (admin/director).** | | `issues` | `/issues` | list, view, create, update, verify, comment, follow/unfollow, verification queue, bulk-verify, delete, quick-assign, **issue links** (`POST /<id>/links` add, `POST /<id>/links/<link_id>/delete` remove, `GET /<id>/link-search` scoped JSON picker — Phase 54), **`POST /bulk`** (bulk assign / status / verify / delete from the list). **verify / bulk-verify / verification-queue are `@issue_manager_required` (admin/director/auditor); delete stays `@supervisor_required` (admin/director).** |
| `notifications` | `/notifications` | list, mark-read, preferences, send-digest (cron), check-sla (cron), cleanup-tokens (cron) | | `notifications` | `/notifications` | list, mark-read, preferences, send-digest (cron), check-sla (cron), cleanup-tokens (cron) |
| `audit` | `/audit` | list (admin only), view, purge | | `audit` | `/audit` | list (admin only), view, purge |
| `reports` | `/reports` | index, facility report, scorecard, CSV/PDF/Excel export, issues-aging, sla-compliance, followup-closure, facility summary PDF | | `reports` | `/reports` | index, facility report, scorecard, CSV/PDF/Excel export, issues-aging, sla-compliance, followup-closure, facility summary PDF |
@@ -1045,7 +1067,24 @@ phase1_projects_roles → phase6_features → phase7_mobile_api → phase8_notif
→ phase50_default_modern → phase50_default_modern
→ phase51_user_notif_matrix → phase51_user_notif_matrix
→ phase52_template_contracts → phase52_template_contracts
→ phase53_followup_assignee ← HEAD → phase53_followup_assignee
→ phase54_issue_links ← HEAD
#### phase54 — link related and duplicate issues
Revision id `phase54_issue_links`. Creates `issue_links` — see §5 `IssueLink`.
**Purely additive.** Nothing reads the table until a person creates a link, so an empty table is exactly today's behaviour and there is nothing to backfill.
Both issue FKs are `ON DELETE CASCADE`, so a direct SQL delete of an issue cannot leave a link pointing at a row that no longer exists (the ORM cascade on `links_from`/`links_to` covers the application path). The index names deliberately match what SQLAlchemy's `index=True` generates — `ix_issue_links_issue_id`, `ix_issue_links_linked_issue_id` — so the schema this migration builds is identical to the one `db.create_all()` builds, down to the index names.
Table-existence check — safe to re-run. `downgrade()` drops the table, discarding every link; no issue is affected, since a link never held state belonging to one.
**Deploy order:**
```bash
flask db upgrade
sudo systemctl restart gunicorn
```
#### phase53 — assign a follow-up to another inspector #### phase53 — assign a follow-up to another inspector
@@ -1755,6 +1794,7 @@ timeout = 30
| 92 | **Bulk deletes: DB rows first, storage files second** | Collect the keys, `db.session.delete()` every row, `commit()`, and only then `storage.delete()`. Deleting files first means a failed/rolled-back commit leaves surviving rows pointing at missing photos. `_collect_inspection_photos()` is shared by the single and bulk inspection delete paths precisely so the two cannot drift — a key missed there is an invisible permanent storage leak. | | 92 | **Bulk deletes: DB rows first, storage files second** | Collect the keys, `db.session.delete()` every row, `commit()`, and only then `storage.delete()`. Deleting files first means a failed/rolled-back commit leaves surviving rows pointing at missing photos. `_collect_inspection_photos()` is shared by the single and bulk inspection delete paths precisely so the two cannot drift — a key missed there is an invisible permanent storage leak. |
| 89 | **`User.CUSTOMER_ROLES` is for ACCOUNT MANAGEMENT; `role == 'customer'` is for CAPABILITY — never swap them** | The inverse of rule 87, and it fails in both directions. Widening a capability check to `CUSTOMER_ROLES` hands a third-party Customer Inspector the customer portal (fail-OPEN, nothing errors). Narrowing an account-management check back to `'customer'` strands every Customer Inspector in a page that no longer lists or edits them (fail-closed, but invisible until someone looks for a missing account). `CUSTOMER_ROLES` / `is_customer_account` appear ONLY in: the `/customers` list query, its route guards, the `auth.list_users` exclusion, **the customer-facing support surface** (`_is_customer_side()` — both roles get the same door, then branch per role for scope and for the AI's system prompt), and **narrowing** uses that WITHHOLD something from an external account (`_assignable_staff_for()` uses it to hide our internal staff — safe direction, and commented as such). Everything else — portal gates, `@customer_required`, `get_customer_scope()`, `notify_customers_for_facility()`, the customer branch of every `app/api/*` scope check — keeps the equality test, because a Customer Inspector is an **inspector** there (rule 87 already routes it correctly). | | 89 | **`User.CUSTOMER_ROLES` is for ACCOUNT MANAGEMENT; `role == 'customer'` is for CAPABILITY — never swap them** | The inverse of rule 87, and it fails in both directions. Widening a capability check to `CUSTOMER_ROLES` hands a third-party Customer Inspector the customer portal (fail-OPEN, nothing errors). Narrowing an account-management check back to `'customer'` strands every Customer Inspector in a page that no longer lists or edits them (fail-closed, but invisible until someone looks for a missing account). `CUSTOMER_ROLES` / `is_customer_account` appear ONLY in: the `/customers` list query, its route guards, the `auth.list_users` exclusion, **the customer-facing support surface** (`_is_customer_side()` — both roles get the same door, then branch per role for scope and for the AI's system prompt), and **narrowing** uses that WITHHOLD something from an external account (`_assignable_staff_for()` uses it to hide our internal staff — safe direction, and commented as such). Everything else — portal gates, `@customer_required`, `get_customer_scope()`, `notify_customers_for_facility()`, the customer branch of every `app/api/*` scope check — keeps the equality test, because a Customer Inspector is an **inspector** there (rule 87 already routes it correctly). |
| 90 | **A per-account notification opt-IN must survive a globally-OFF column** | `notify_by_matrix()` skips a role column early when the matrix says off. For the two customer columns that early `continue` has to also ask whether anyone opted in (`any(overrides.values())`), or the override saves, displays as on, and never sends — a silent failure with no error anywhere. Equally, `notify_customers_for_facility()` re-queries recipients from assignment rows, so `notify_by_matrix()` must hand it `allowed_user_ids` or the facility-scoped path bypasses every override. Both halves are needed; either one alone leaves a hole. See §11. | | 90 | **A per-account notification opt-IN must survive a globally-OFF column** | `notify_by_matrix()` skips a role column early when the matrix says off. For the two customer columns that early `continue` has to also ask whether anyone opted in (`any(overrides.values())`), or the override saves, displays as on, and never sends — a silent failure with no error anywhere. Equally, `notify_customers_for_facility()` re-queries recipients from assignment rows, so `notify_by_matrix()` must hand it `allowed_user_ids` or the facility-scoped path bypasses every override. Both halves are needed; either one alone leaves a hole. See §11. |
| 99 | **An issue link is a pointer to another issue — filter it by scope on ALL THREE surfaces** | A link exposes the far issue's id, description, facility and status, so an unfiltered panel lets a customer read an issue at a facility they hold no assignment to, simply because one of our staff linked it. Three surfaces have to hold the line and only one of them is a real boundary: `_readable_links()` filters what the panel RENDERS, `link_search()` scopes what the picker FINDS, and `add_link()` re-checks on POST — the search is a convenience and must never be trusted as the gate. All three resolve scope through `_viewer_facility_scope()` / `_issue_in_scope()`, the same pair `issues.view()` now uses, so the panel cannot end up more permissive than the page it sits on (the rule 93 lesson, applied before it could bite). `_issue_in_scope` takes a resolved scope rather than a user so filtering a list costs one assignment query, not one per row. A link to an issue outside your scope reports "not found", never "access denied" — whether another customer's issue exists is not something the link box should confirm. |
| 81 | **Photo timestamp/geo overlay is burned at UPLOAD, never on `PATCH /issues/<id>/photos`** | That PATCH receives only path strings — the bytes are already in storage and the payload carries no capture metadata. Burning there would need a read-modify-write per key plus an overwrite-in-place primitive (`storage.save()` mints a NEW uuid key, and §22 requires key == DB path), and would risk a **double burn** since the endpoint is deliberately idempotent/retry-safe (rule 45). Stamp in `POST /photos/upload`, where the raw bytes + EXIF are in hand and each call writes exactly one already-stamped object. Stamping failures must always fall back to storing the ORIGINAL bytes — never lose a photo to a stamping bug. See §23. | | 81 | **Photo timestamp/geo overlay is burned at UPLOAD, never on `PATCH /issues/<id>/photos`** | That PATCH receives only path strings — the bytes are already in storage and the payload carries no capture metadata. Burning there would need a read-modify-write per key plus an overwrite-in-place primitive (`storage.save()` mints a NEW uuid key, and §22 requires key == DB path), and would risk a **double burn** since the endpoint is deliberately idempotent/retry-safe (rule 45). Stamp in `POST /photos/upload`, where the raw bytes + EXIF are in hand and each call writes exactly one already-stamped object. Stamping failures must always fall back to storing the ORIGINAL bytes — never lose a photo to a stamping bug. See §23. |
--- ---
+136
View File
@@ -43,6 +43,116 @@ class IssueFollower(db.Model):
return f'<IssueFollower issue={self.issue_id} user={self.user_id}>' return f'<IssueFollower issue={self.issue_id} user={self.user_id}>'
# ── Issue Link ────────────────────────────────────────────────────────────────
# Connects two issues so staff can jump between a duplicate and the original, or
# between issues that are simply about the same thing.
class IssueLink(db.Model):
"""One directed link between two issues, displayed on BOTH of them.
Only one row is stored per pair. The stored direction carries meaning for
'duplicate' issue_id is a duplicate OF linked_issue_id so the two issues
read the same row differently:
on issue_id -> "Duplicate of #B"
on linked_issue_id -> "Duplicated by #A"
'related' is symmetric and reads "Related to" from either side.
Storing one row rather than a mirrored pair is what keeps the direction
unambiguous and makes unlinking a single delete. The cost is that uniqueness
cannot be expressed by the UniqueConstraint alone: (A,B) and (B,A) are
distinct rows to the database but the same link to a person, so the
duplicate check has to look in both directions. exists_between() is that
check, and it is the only thing callers should use.
A link is PURELY NAVIGATIONAL. Marking a duplicate does not touch either
issue's status, SLA, assignee or followers — closing the duplicate stays a
deliberate, separate action.
"""
__tablename__ = 'issue_links'
TYPE_DUPLICATE = 'duplicate'
TYPE_RELATED = 'related'
# How each link type reads from the two sides, keyed by (type, is_source).
LABELS = {
('duplicate', True): 'Duplicate of',
('duplicate', False): 'Duplicated by',
('related', True): 'Related to',
('related', False): 'Related to',
}
# Offered in the "Link an issue" picker. The value is what gets stored; the
# phrasing is from the point of view of the issue being viewed.
TYPE_CHOICES = [
('duplicate', 'Duplicate of'),
('related', 'Related to'),
]
id = db.Column(db.Integer, primary_key=True)
issue_id = db.Column(db.Integer,
db.ForeignKey('issues.id', ondelete='CASCADE'),
nullable=False, index=True)
linked_issue_id = db.Column(db.Integer,
db.ForeignKey('issues.id', ondelete='CASCADE'),
nullable=False, index=True)
link_type = db.Column(db.Enum('duplicate', 'related'),
nullable=False, default='related')
created_by = db.Column(db.Integer,
db.ForeignKey('users.id', ondelete='SET NULL'),
nullable=True)
created_at = db.Column(db.DateTime, default=now_eastern, nullable=False)
__table_args__ = (
# Catches the exact-duplicate row at the database level. The REVERSE
# direction is caught by exists_between() — see the class docstring.
db.UniqueConstraint('issue_id', 'linked_issue_id', name='uq_issue_link'),
)
# BOTH relationships must pin foreign_keys: two FKs from this table to
# issues leave the join condition ambiguous otherwise, and the mapper raises
# on first ORM USE rather than at import — the app starts cleanly and then
# every request 500s (CLAUDE.md rule 86).
issue = db.relationship('Issue', foreign_keys=[issue_id],
back_populates='links_from')
linked_issue = db.relationship('Issue', foreign_keys=[linked_issue_id],
back_populates='links_to')
creator = db.relationship('User', foreign_keys=[created_by])
def label_for(self, viewing_issue_id):
"""How this link reads on the issue currently being viewed."""
return self.LABELS[(self.link_type, self.issue_id == viewing_issue_id)]
def other_issue(self, viewing_issue_id):
"""The issue at the far end of this link from the one being viewed."""
return (self.linked_issue if self.issue_id == viewing_issue_id
else self.issue)
@staticmethod
def exists_between(issue_id, other_id):
"""True when the two issues are already linked, in EITHER direction.
The UniqueConstraint only covers the stored direction, so this is what
stops #A being linked to #B and then #B linked back to #A as a second,
contradictory row.
"""
return db.session.query(
IssueLink.query.filter(
db.or_(
db.and_(IssueLink.issue_id == issue_id,
IssueLink.linked_issue_id == other_id),
db.and_(IssueLink.issue_id == other_id,
IssueLink.linked_issue_id == issue_id),
)
).exists()
).scalar()
def __repr__(self):
return (f'<IssueLink {self.issue_id} {self.link_type} '
f'{self.linked_issue_id}>')
class Issue(db.Model): class Issue(db.Model):
__tablename__ = 'issues' __tablename__ = 'issues'
@@ -115,10 +225,36 @@ class Issue(db.Model):
followers = db.relationship('IssueFollower', back_populates='issue', followers = db.relationship('IssueFollower', back_populates='issue',
cascade='all, delete-orphan', lazy='dynamic') cascade='all, delete-orphan', lazy='dynamic')
# An issue link is stored once and shown on both issues, so each issue has
# rows pointing OUT of it and rows pointing AT it. Deleting an issue must
# take its links with it from BOTH sides, or the surviving issue keeps a row
# referencing one that no longer exists.
links_from = db.relationship('IssueLink', back_populates='issue',
foreign_keys='IssueLink.issue_id',
cascade='all, delete-orphan', lazy='dynamic')
links_to = db.relationship('IssueLink', back_populates='linked_issue',
foreign_keys='IssueLink.linked_issue_id',
cascade='all, delete-orphan', lazy='dynamic')
def is_followed_by(self, user): def is_followed_by(self, user):
"""Return True if the given user is currently following this issue.""" """Return True if the given user is currently following this issue."""
return self.followers.filter_by(user_id=user.id).first() is not None return self.followers.filter_by(user_id=user.id).first() is not None
def all_links(self):
"""Every link touching this issue, from both directions, newest first.
The two relationships are a storage detail a link is one thing to the
person reading it, so callers get a single list and ask each row how it
reads via label_for() / other_issue().
Nothing here filters by permission. The caller MUST drop links whose far
end the viewer cannot access, or a link becomes a way to read another
customer's issue. See _readable_links() in routes/issues.py.
"""
links = list(self.links_from) + list(self.links_to)
links.sort(key=lambda link: link.created_at, reverse=True)
return links
# Human-readable label for the handler category (phase35). # Human-readable label for the handler category (phase35).
# Perspective-neutral wording so it reads the same for staff and customers. # Perspective-neutral wording so it reads the same for staff and customers.
HANDLER_LABELS = { HANDLER_LABELS = {
+249 -13
View File
@@ -6,7 +6,7 @@ from flask import (Blueprint, render_template, redirect, url_for,
flash, request, current_app, jsonify, abort, Response) flash, request, current_app, jsonify, abort, Response)
from flask_login import login_required, current_user from flask_login import login_required, current_user
from app import db from app import db
from app.models.issue import Issue, IssueComment, IssueFollower from app.models.issue import Issue, IssueComment, IssueFollower, IssueLink
from app.models.facility import Facility, Area from app.models.facility import Facility, Area
from app.models.user import User from app.models.user import User
from app.models.notification import ( from app.models.notification import (
@@ -85,6 +85,44 @@ class _SLAFilteredPage:
# ── Issue read access ─────────────────────────────────────────────────────────
# One definition of "may this person open this issue", used by the detail view,
# by the linked-issues panel, and by the link picker's search. They must not
# drift: the picker is what a person searches, but the panel is what actually
# renders another issue's description, and the POST is the real boundary.
def _viewer_facility_scope(user):
"""Facility ids this user is confined to, or None when unrestricted.
Returns a LIST (possibly empty) for the two scoped role groups and None for
everyone else. Empty list and None mean opposite things [] is "no access
to anything", None is "no restriction" — so callers must test `is None`
rather than truthiness (CLAUDE.md rule 57's failure mode).
"""
if user.is_inspector: # rule 87 — never role == 'inspector'
return get_inspector_scope(user) or []
if user.role == 'customer': # rule 89 — capability check, exact match
return get_customer_scope(user) or []
return None
def _issue_in_scope(issue, scope_ids):
"""Whether one issue falls inside an already-resolved facility scope.
Takes the scope rather than the user so a caller filtering a list of issues
resolves it once instead of re-querying the assignment tables per row.
"""
if scope_ids is None:
return True
facility = issue.resolved_facility
return facility is not None and facility.id in scope_ids
def _issue_readable_by(issue, user):
"""Single-issue convenience wrapper around the two helpers above."""
return _issue_in_scope(issue, _viewer_facility_scope(user))
def _assignee_label(user): def _assignee_label(user):
"""Dropdown label for an assignee. """Dropdown label for an assignee.
@@ -414,18 +452,14 @@ def view(issue_id):
if issue is None: if issue is None:
abort(404) abort(404)
if current_user.is_inspector: # Scope gate — see _issue_readable_by(). This was two inline blocks that the
fids = get_inspector_scope(current_user) # linked-issues panel would have had to reproduce a third time; it is now
facility = issue.resolved_facility # one definition so the panel cannot end up more permissive than the page.
if not fids or not facility or facility.id not in fids: if not _issue_readable_by(issue, current_user):
flash('Access denied.', 'danger') flash('Access denied.', 'danger')
return redirect(url_for('issues.index')) return redirect(url_for('issues.index'))
if current_user.role == 'customer': if current_user.role == 'customer':
cids = get_customer_scope(current_user) or []
facility = issue.resolved_facility
if not facility or facility.id not in cids:
flash('Access denied.', 'danger')
return redirect(url_for('issues.index'))
if request.method == 'POST': if request.method == 'POST':
# Customers may only add a comment, and only on issues they follow or reported # Customers may only add a comment, and only on issues they follow or reported
can_comment = (issue.is_followed_by(current_user) or issue.reported_by == current_user.id) can_comment = (issue.is_followed_by(current_user) or issue.reported_by == current_user.id)
@@ -710,7 +744,209 @@ def view(issue_id):
form=form, form=form,
comments=comments, comments=comments,
comments_open=comments_open, comments_open=comments_open,
is_following=is_following) is_following=is_following,
# Already filtered to links whose far end this viewer
# may open — see _readable_links().
issue_links=_readable_links(issue, current_user),
link_types=IssueLink.TYPE_CHOICES,
can_manage_links=_can_manage_links(issue, current_user))
# ── Issue links ───────────────────────────────────────────────────────────────
# Connect a duplicate to its original, or two issues about the same thing, so
# whoever picks one up can reach the other. Links are purely navigational: they
# never touch status, SLA, assignee or followers on either issue.
def _can_manage_links(issue, user):
"""Who may add or remove a link on this issue.
Deliberately the SAME set as the page's `can_edit` (the Update Issue panel):
admin / director / auditor, or the person the issue is assigned to. Keeping
the two identical means the panel's buttons and this gate cannot disagree —
the alternative is a second, slightly different rule that nobody remembers.
Widening it (to project_manager, or to the reporter) is a one-line change
here, but change `can_edit` in issues/view.html at the same time.
"""
return (user.role in ('admin', 'director', 'auditor')
or issue.assigned_to == user.id)
def _readable_links(issue, user):
"""Links on this issue whose FAR END the viewer may also open.
A link is a pointer to another issue's id, description and facility, so an
unfiltered panel would let a customer read an issue at a facility they have
no assignment to simply because one of our staff linked it. The scope is
resolved once for the whole list rather than per row.
Returns a list of (link, other_issue, label) ready for the template.
"""
scope = _viewer_facility_scope(user)
visible = []
for link in issue.all_links():
other = link.other_issue(issue.id)
if other is None or not _issue_in_scope(other, scope):
continue
visible.append((link, other, link.label_for(issue.id)))
return visible
@bp.route('/<int:issue_id>/links', methods=['POST'])
@login_required
def add_link(issue_id):
"""Link this issue to another one."""
issue = db.session.get(Issue, issue_id)
if issue is None:
abort(404)
if not _issue_readable_by(issue, current_user):
abort(403)
if not _can_manage_links(issue, current_user):
abort(403)
link_type = request.form.get('link_type', '')
if link_type not in (IssueLink.TYPE_DUPLICATE, IssueLink.TYPE_RELATED):
flash('Choose how the two issues are related.', 'warning')
return redirect(_view_url(issue_id))
raw_target = (request.form.get('linked_issue_id') or '').strip().lstrip('#')
if not raw_target.isdigit():
flash('Enter the number of the issue to link, e.g. 412.', 'warning')
return redirect(_view_url(issue_id))
target_id = int(raw_target)
if target_id == issue.id:
flash('An issue cannot be linked to itself.', 'warning')
return redirect(_view_url(issue_id))
target = db.session.get(Issue, target_id)
# A 404 and a 403 are the same message here on purpose: whether an issue
# outside your scope EXISTS is not something the link box should confirm.
if target is None or not _issue_readable_by(target, current_user):
flash(f'Issue #{target_id} was not found.', 'warning')
return redirect(_view_url(issue_id))
if IssueLink.exists_between(issue.id, target.id):
flash(f'Issue #{issue.id} and #{target.id} are already linked.', 'info')
return redirect(_view_url(issue_id))
link = IssueLink(
issue_id = issue.id,
linked_issue_id = target.id,
link_type = link_type,
created_by = current_user.id,
)
db.session.add(link)
db.session.commit()
log_action(ACTION_UPDATE, 'Issue', issue.id, f'#{issue.id}',
f'linked to #{target.id} as {link_type}')
current_app.logger.info(
'ISSUE LINK | issue_id=%s | linked_issue_id=%s | type=%s | user=%s',
issue.id, target.id, link_type, current_user.username,
)
flash(f'Issue #{issue.id} is now linked to #{target.id}.', 'success')
return redirect(_view_url(issue_id))
@bp.route('/<int:issue_id>/links/<int:link_id>/delete', methods=['POST'])
@login_required
def remove_link(issue_id, link_id):
"""Remove a link. Either end of it may do this."""
issue = db.session.get(Issue, issue_id)
if issue is None:
abort(404)
if not _issue_readable_by(issue, current_user):
abort(403)
if not _can_manage_links(issue, current_user):
abort(403)
link = db.session.get(IssueLink, link_id)
# The link must actually touch THIS issue. Without the check, anyone able to
# manage links on any one issue could delete a link between two others by
# posting its id here.
if link is None or issue.id not in (link.issue_id, link.linked_issue_id):
flash('That link no longer exists.', 'info')
return redirect(_view_url(issue_id))
other_id = link.linked_issue_id if link.issue_id == issue.id else link.issue_id
db.session.delete(link)
db.session.commit()
log_action(ACTION_UPDATE, 'Issue', issue.id, f'#{issue.id}',
f'unlinked from #{other_id}')
current_app.logger.info(
'ISSUE UNLINK | issue_id=%s | linked_issue_id=%s | user=%s',
issue.id, other_id, current_user.username,
)
flash(f'Removed the link to issue #{other_id}.', 'info')
return redirect(_view_url(issue_id))
@bp.route('/<int:issue_id>/link-search')
@login_required
def link_search(issue_id):
"""JSON candidates for the link picker.
Scoped exactly like the issue list, so an inspector or customer can only
find issues they could already open searching must not become a way to
enumerate another contract's issues. The results are a convenience; the POST
in add_link() re-checks access and is the real boundary.
"""
issue = db.session.get(Issue, issue_id)
if issue is None:
abort(404)
if not _issue_readable_by(issue, current_user):
abort(403)
term = (request.args.get('q') or '').strip().lstrip('#')
if len(term) < 1:
return jsonify({'results': []})
q = (
Issue.query
.outerjoin(Area, Issue.area_id == Area.id)
.options(joinedload(Issue.facility), contains_eager(Issue.area))
.filter(Issue.id != issue.id)
)
scope = _viewer_facility_scope(current_user)
if scope is not None:
if not scope:
return jsonify({'results': []})
q = q.filter(db.or_(
Issue.facility_id.in_(scope),
db.and_(Issue.area_id.isnot(None), Area.facility_id.in_(scope)),
))
# Exclude issues already linked in either direction — offering them only
# produces an "already linked" flash.
linked_ids = {other.id for _l, other, _lbl in _readable_links(issue, current_user)}
if linked_ids:
q = q.filter(Issue.id.notin_(linked_ids))
if term.isdigit():
# A number is almost always an issue number, so match the id first and
# fall back to the description for things like "Room 204".
q = q.filter(db.or_(Issue.id == int(term),
Issue.description.ilike(f'%{term}%')))
else:
q = q.filter(Issue.description.ilike(f'%{term}%'))
matches = q.order_by(Issue.reported_at.desc()).limit(10).all()
return jsonify({'results': [
{
'id': i.id,
'description': (i.description or '')[:110],
'status': (i.status or '').replace('_', ' ').title(),
'severity': (i.severity or '').title(),
'location': (i.area.name if i.area
else i.resolved_facility.name if i.resolved_facility
else ''),
'reported_at': i.reported_at.strftime('%Y-%m-%d') if i.reported_at else '',
}
for i in matches
]})
# ── Follow ──────────────────────────────────────────────────────────────────── # ── Follow ────────────────────────────────────────────────────────────────────
+264
View File
@@ -232,6 +232,84 @@
</div> </div>
</div> </div>
{# ── Linked issues ──────────────────────────────────────────────────────
Duplicates and related issues, so whoever picks this one up can reach the
others. `issue_links` arrives already filtered to links whose far end this
viewer may open (_readable_links) — do NOT add links from the model
directly here, or a customer sees an issue at a facility they have no
assignment to. Links are navigational only: nothing here changes status,
SLA, assignee or followers on either issue. #}
<div class="card shadow-sm mb-4" id="linked-issues-section">
<div class="card-header bg-light d-flex justify-content-between align-items-center">
<h6 class="mb-0">
<i class="bi bi-link-45deg me-1"></i>Linked Issues
<span class="badge bg-secondary rounded-pill ms-1">{{ issue_links|length }}</span>
</h6>
{% if can_manage_links %}
<button type="button" class="btn btn-sm btn-outline-primary"
data-bs-toggle="modal" data-bs-target="#linkIssueModal">
<i class="bi bi-plus-lg me-1"></i>Link an Issue
</button>
{% endif %}
</div>
<div class="card-body py-2">
{% if issue_links %}
<div class="list-group list-group-flush">
{% for link, other, label in issue_links %}
<div class="list-group-item px-0 py-2 d-flex align-items-start gap-2 flex-wrap">
<span class="badge {{ 'bg-warning text-dark' if link.link_type == 'duplicate' else 'bg-info text-dark' }} mt-1"
style="min-width:7.5rem;">{{ label }}</span>
<div class="flex-grow-1" style="min-width:14rem;">
<a href="{{ url_for('issues.view', issue_id=other.id, next=back_url) }}"
class="fw-semibold text-decoration-none">#{{ other.id }}</a>
<span class="text-muted small ms-1">
{{ other.area.name if other.area
else other.resolved_facility.name if other.resolved_facility else '—' }}
</span>
<div class="small text-muted text-truncate" style="max-width:38rem;">
{{ other.description }}
</div>
</div>
<div class="d-flex align-items-center gap-1 mt-1">
<span class="badge bg-{{ 'danger' if other.severity in ['critical','high']
else 'warning text-dark' if other.severity == 'medium'
else 'secondary' }}">{{ other.severity|title }}</span>
<span class="badge bg-{{ 'success' if other.status == 'resolved'
else 'info text-dark' if other.status == 'pending_verification'
else 'light text-dark' }}">
{{ other.status|replace('_',' ')|title }}
</span>
{% if can_manage_links %}
<form method="POST" class="mb-0 ms-1"
action="{{ url_for('issues.remove_link', issue_id=issue.id, link_id=link.id) }}"
onsubmit="return confirm('Remove the link between #{{ issue.id }} and #{{ other.id }}? Neither issue is changed or deleted.');">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<input type="hidden" name="next" value="{{ back_url }}">
<button type="submit" class="btn btn-sm btn-link text-muted p-0 px-1"
title="Remove this link">
<i class="bi bi-x-lg"></i>
</button>
</form>
{% endif %}
</div>
</div>
{% endfor %}
</div>
{% else %}
<p class="text-muted small mb-0 py-1">
<i class="bi bi-info-circle me-1"></i>
No linked issues.
{% if can_manage_links %}
Use <strong>Link an Issue</strong> to point at a duplicate or a related issue.
{% endif %}
</p>
{% endif %}
</div>
</div>
{# ── Comments ───────────────────────────────────────────────────────── #} {# ── Comments ───────────────────────────────────────────────────────── #}
<div class="card shadow-sm mb-4" id="comments-section"> <div class="card shadow-sm mb-4" id="comments-section">
<div class="card-header bg-light d-flex justify-content-between align-items-center"> <div class="card-header bg-light d-flex justify-content-between align-items-center">
@@ -578,6 +656,77 @@
{% endif %} {% endif %}
</div> </div>
{# ── Link an issue ────────────────────────────────────────────────────────────
Search is scoped server-side to issues this viewer could already open, so the
picker can never be used to enumerate another contract's issues. The POST
re-checks access — the search is only a convenience. #}
{% if can_manage_links %}
<div class="modal fade" id="linkIssueModal" tabindex="-1"
aria-labelledby="linkIssueModalLabel" aria-hidden="true">
<div class="modal-dialog modal-dialog-centered">
<div class="modal-content">
<form method="POST" action="{{ url_for('issues.add_link', issue_id=issue.id) }}">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<input type="hidden" name="next" value="{{ back_url }}">
<div class="modal-header">
<h5 class="modal-title" id="linkIssueModalLabel">
<i class="bi bi-link-45deg me-1"></i>Link an issue to #{{ issue.id }}
</h5>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
</div>
<div class="modal-body">
<div class="mb-3">
<label class="form-label small fw-semibold" for="linkTypeSelect">
How are they related?
</label>
<select name="link_type" id="linkTypeSelect" class="form-select form-select-sm">
{% for value, label in link_types %}
<option value="{{ value }}">
#{{ issue.id }} is a <strong>{{ label|lower }}</strong>
</option>
{% endfor %}
</select>
<div class="form-text">
Linking is for navigation only — neither issue's status, SLA or
assignee changes.
</div>
</div>
<div class="mb-2">
<label class="form-label small fw-semibold" for="linkIssueSearch">
Which issue?
</label>
<input type="text" class="form-control form-control-sm" id="linkIssueSearch"
autocomplete="off" placeholder="Issue number, or words from the description…">
<input type="hidden" name="linked_issue_id" id="linkIssueId">
</div>
{# Chosen issue, shown once picked so nobody submits a mistyped number #}
<div id="linkIssueChosen" class="alert alert-primary py-2 small d-none mb-2">
<span id="linkIssueChosenText"></span>
<button type="button" class="btn btn-sm btn-link p-0 ms-2" id="linkIssueClear">change</button>
</div>
<div id="linkIssueResults" class="list-group small" style="max-height:16rem; overflow-y:auto;"></div>
<div id="linkIssueEmpty" class="text-muted small d-none py-2">
No matching issue you can access.
</div>
</div>
<div class="modal-footer">
<button type="button" class="btn btn-secondary btn-sm" data-bs-dismiss="modal">Cancel</button>
<button type="submit" class="btn btn-primary btn-sm" id="linkIssueSubmit" disabled>
<i class="bi bi-link-45deg me-1"></i>Link Issue
</button>
</div>
</form>
</div>
</div>
</div>
{% endif %}
{% if current_user.role in ['admin', 'director'] %} {% if current_user.role in ['admin', 'director'] %}
<div class="modal fade" id="deleteIssueModal" tabindex="-1" aria-labelledby="deleteIssueModalLabel" aria-hidden="true"> <div class="modal fade" id="deleteIssueModal" tabindex="-1" aria-labelledby="deleteIssueModalLabel" aria-hidden="true">
<div class="modal-dialog modal-dialog-centered"> <div class="modal-dialog modal-dialog-centered">
@@ -634,6 +783,121 @@
} }
} }
// ── Link-an-issue picker ────────────────────────────────────────────────
// Type a number or some words, pick from the scoped results, submit. The
// hidden linked_issue_id is only ever set by CHOOSING a result, so the
// number posted is always one the server just confirmed this user can see.
//
// Every result field is written with textContent / createTextNode, never
// innerHTML: `description` is text a person typed and would otherwise be
// an XSS hole straight into the page of whoever opens the picker.
var linkSearch = document.getElementById('linkIssueSearch');
if (linkSearch) {
var linkResults = document.getElementById('linkIssueResults');
var linkEmpty = document.getElementById('linkIssueEmpty');
var linkIdField = document.getElementById('linkIssueId');
var linkChosen = document.getElementById('linkIssueChosen');
var linkChosenText = document.getElementById('linkIssueChosenText');
var linkClear = document.getElementById('linkIssueClear');
var linkSubmit = document.getElementById('linkIssueSubmit');
var searchTimer = null;
var searchSeq = 0;
function clearChoice() {
linkIdField.value = '';
linkSubmit.disabled = true;
linkChosen.classList.add('d-none');
linkSearch.classList.remove('d-none');
}
function choose(item) {
linkIdField.value = item.id;
linkSubmit.disabled = false;
linkChosenText.textContent =
'#' + item.id + ' — ' + item.location + ' — ' + item.description;
linkChosen.classList.remove('d-none');
linkSearch.classList.add('d-none');
linkResults.innerHTML = '';
linkEmpty.classList.add('d-none');
}
function renderResults(items) {
linkResults.innerHTML = '';
linkEmpty.classList.toggle('d-none', items.length > 0);
items.forEach(function (item) {
var row = document.createElement('button');
row.type = 'button';
row.className = 'list-group-item list-group-item-action py-2';
var head = document.createElement('div');
head.className = 'd-flex justify-content-between gap-2';
var num = document.createElement('span');
num.className = 'fw-semibold';
num.textContent = '#' + item.id + ' · ' + item.location;
var meta = document.createElement('span');
meta.className = 'text-muted';
meta.textContent = item.severity + ' · ' + item.status +
(item.reported_at ? ' · ' + item.reported_at : '');
head.appendChild(num);
head.appendChild(meta);
var desc = document.createElement('div');
desc.className = 'text-muted text-truncate';
desc.textContent = item.description;
row.appendChild(head);
row.appendChild(desc);
row.addEventListener('click', function () { choose(item); });
linkResults.appendChild(row);
});
}
function runSearch() {
var term = linkSearch.value.trim();
if (!term) {
linkResults.innerHTML = '';
linkEmpty.classList.add('d-none');
return;
}
// Responses can arrive out of order; only the newest one may render.
var seq = ++searchSeq;
fetch('{{ url_for("issues.link_search", issue_id=issue.id) }}?q=' +
encodeURIComponent(term), { headers: { 'Accept': 'application/json' } })
.then(function (res) { return res.ok ? res.json() : { results: [] }; })
.then(function (data) {
if (seq !== searchSeq) { return; }
renderResults(data.results || []);
})
.catch(function () {
if (seq !== searchSeq) { return; }
renderResults([]);
});
}
linkSearch.addEventListener('input', function () {
clearTimeout(searchTimer);
searchTimer = setTimeout(runSearch, 250);
});
// The picker lives inside a form — Enter would submit it with no issue
// chosen instead of searching.
linkSearch.addEventListener('keydown', function (ev) {
if (ev.key === 'Enter') {
ev.preventDefault();
clearTimeout(searchTimer);
runSearch();
}
});
linkClear.addEventListener('click', function () {
clearChoice();
linkSearch.value = '';
linkSearch.focus();
});
}
// ── "Handled By" — show the relevant sub-block (facility vs vendor) and // ── "Handled By" — show the relevant sub-block (facility vs vendor) and
// relabel the assignee as a follow-up owner for facility/vendor. ── // relabel the assignee as a follow-up owner for facility/vendor. ──
var handlerSelect = document.getElementById('handler_type_select'); var handlerSelect = document.getElementById('handler_type_select');
@@ -0,0 +1,72 @@
"""phase54 — link related and duplicate issues
Creates `issue_links`: one row per connection between two issues, displayed on
both of them. See the IssueLink docstring in app/models/issue.py for why the
direction is stored once rather than mirrored.
Purely additive. Nothing reads the table until a person creates a link, so an
empty table is exactly today's behaviour and there is nothing to backfill.
Both FKs use ON DELETE CASCADE so deleting an issue takes its links with it from
either end a surviving link pointing at a deleted issue would render a dead
row on the other issue's page. The ORM cascade on Issue.links_from/links_to
covers the application path; this covers a direct SQL delete.
Table-existence check safe to re-run.
Revision ID: phase54_issue_links
Revises: phase53_followup_assignee
"""
from alembic import op
import sqlalchemy as sa
# Revision ids must be <= 32 chars — alembic_version.version_num is VARCHAR(32).
revision = 'phase54_issue_links'
down_revision = 'phase53_followup_assignee'
branch_labels = None
depends_on = None
def _table_exists(conn, name):
return conn.execute(sa.text("""
SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLES
WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = :t
"""), {'t': name}).scalar() > 0
def upgrade():
conn = op.get_bind()
if _table_exists(conn, 'issue_links'):
return
op.execute(sa.text("""
CREATE TABLE issue_links (
id INT AUTO_INCREMENT PRIMARY KEY,
issue_id INT NOT NULL,
linked_issue_id INT NOT NULL,
link_type ENUM('duplicate','related') NOT NULL DEFAULT 'related',
created_by INT NULL,
created_at DATETIME NOT NULL,
CONSTRAINT uq_issue_link UNIQUE (issue_id, linked_issue_id),
-- Names match what SQLAlchemy's index=True generates, so the
-- schema this migration builds and the one db.create_all() builds
-- are identical down to the index names.
INDEX ix_issue_links_issue_id (issue_id),
INDEX ix_issue_links_linked_issue_id (linked_issue_id),
CONSTRAINT fk_issue_links_issue
FOREIGN KEY (issue_id) REFERENCES issues (id) ON DELETE CASCADE,
CONSTRAINT fk_issue_links_linked
FOREIGN KEY (linked_issue_id) REFERENCES issues (id) ON DELETE CASCADE,
CONSTRAINT fk_issue_links_creator
FOREIGN KEY (created_by) REFERENCES users (id) ON DELETE SET NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
"""))
def downgrade():
conn = op.get_bind()
if _table_exists(conn, 'issue_links'):
op.execute(sa.text('DROP TABLE issue_links'))
+485
View File
@@ -0,0 +1,485 @@
"""
Tests for issue linking (duplicate / related).
Two things are worth testing here. The first is the link semantics: one stored
row read from both sides, no self-links, no double-linking in either direction.
The second, and the reason this file is long, is SCOPE. A link is a pointer to
another issue's id, description and facility, so it is a potential way to read
an issue you have no access to. Three surfaces have to hold that line
independently -- the panel that renders links, the search that finds candidates,
and the POST that creates them -- and only the POST is a real boundary.
"""
import pytest
from app import db as _db
from app.models.issue import Issue, IssueLink
from app.models.project import Project, CustomerAssignment
from app.models.inspector_assignment import InspectorAssignment
from app.utils.time_utils import now_eastern
# ── Fixtures ─────────────────────────────────────────────────────────────────
@pytest.fixture
def make_issue(db):
def _make(facility=None, area=None, severity='high', status='open', **kw):
issue = Issue(
facility_id=facility.id if facility else None,
area_id=area.id if area else None,
severity=severity,
description=kw.pop('description', 'a leak under the sink'),
status=status,
reported_at=now_eastern(),
)
for k, v in kw.items():
setattr(issue, k, v)
db.session.add(issue)
db.session.commit()
return issue
return _make
@pytest.fixture
def make_project(db):
def _make(name='Contract A'):
proj = Project(name=name, active=True)
db.session.add(proj)
db.session.commit()
return proj
return _make
@pytest.fixture
def link_url():
def _make(issue):
return f'/issues/{issue.id}/links'
return _make
# ── Model semantics ──────────────────────────────────────────────────────────
def test_a_link_reads_differently_from_each_end(app, db, make_issue, make_facility):
"""One stored row; 'duplicate' is directional and says so on both issues."""
facility = make_facility()
dupe = make_issue(facility=facility)
orig = make_issue(facility=facility)
db.session.add(IssueLink(issue_id=dupe.id, linked_issue_id=orig.id,
link_type='duplicate'))
db.session.commit()
link = IssueLink.query.one()
assert link.label_for(dupe.id) == 'Duplicate of'
assert link.label_for(orig.id) == 'Duplicated by'
assert link.other_issue(dupe.id).id == orig.id
assert link.other_issue(orig.id).id == dupe.id
def test_related_reads_the_same_from_both_ends(app, db, make_issue, make_facility):
facility = make_facility()
a, b = make_issue(facility=facility), make_issue(facility=facility)
db.session.add(IssueLink(issue_id=a.id, linked_issue_id=b.id,
link_type='related'))
db.session.commit()
link = IssueLink.query.one()
assert link.label_for(a.id) == 'Related to'
assert link.label_for(b.id) == 'Related to'
def test_one_row_appears_on_both_issues(app, db, make_issue, make_facility):
"""all_links() merges the two storage directions into one list."""
facility = make_facility()
a, b = make_issue(facility=facility), make_issue(facility=facility)
db.session.add(IssueLink(issue_id=a.id, linked_issue_id=b.id,
link_type='related'))
db.session.commit()
assert len(a.all_links()) == 1
assert len(b.all_links()) == 1
assert IssueLink.query.count() == 1
def test_exists_between_is_direction_agnostic(app, db, make_issue, make_facility):
"""The stored UniqueConstraint only covers one direction -- this covers both."""
facility = make_facility()
a, b = make_issue(facility=facility), make_issue(facility=facility)
db.session.add(IssueLink(issue_id=a.id, linked_issue_id=b.id,
link_type='related'))
db.session.commit()
assert IssueLink.exists_between(a.id, b.id)
assert IssueLink.exists_between(b.id, a.id)
assert not IssueLink.exists_between(a.id, 99999)
def test_deleting_an_issue_removes_its_links_from_both_sides(
app, db, make_issue, make_facility):
"""A surviving link would render a dead row on the other issue's page."""
facility = make_facility()
a, b, c = (make_issue(facility=facility) for _ in range(3))
db.session.add(IssueLink(issue_id=a.id, linked_issue_id=b.id, link_type='related'))
db.session.add(IssueLink(issue_id=c.id, linked_issue_id=a.id, link_type='duplicate'))
db.session.commit()
assert IssueLink.query.count() == 2
db.session.delete(a) # the path both the single and bulk delete use
db.session.commit()
assert IssueLink.query.count() == 0
assert b.all_links() == []
assert c.all_links() == []
# ── Creating links through the route ─────────────────────────────────────────
def test_admin_can_link_two_issues(client, login, make_user, make_facility,
make_issue, link_url):
facility = make_facility()
a, b = make_issue(facility=facility), make_issue(facility=facility)
login(make_user(role='admin'))
res = client.post(link_url(a),
data={'link_type': 'duplicate', 'linked_issue_id': str(b.id)},
follow_redirects=True)
assert res.status_code == 200
link = IssueLink.query.one()
assert (link.issue_id, link.linked_issue_id) == (a.id, b.id)
assert link.link_type == 'duplicate'
def test_a_leading_hash_is_accepted(client, login, make_user, make_facility,
make_issue, link_url):
"""People type '#412' because that is how the id is shown everywhere."""
facility = make_facility()
a, b = make_issue(facility=facility), make_issue(facility=facility)
login(make_user(role='admin'))
client.post(link_url(a),
data={'link_type': 'related', 'linked_issue_id': f'#{b.id}'},
follow_redirects=True)
assert IssueLink.query.count() == 1
def test_linking_does_not_touch_either_issue(client, login, make_user,
make_facility, make_issue, link_url):
"""The chosen design: a link is navigation, not a workflow action."""
facility = make_facility()
a = make_issue(facility=facility, status='open')
b = make_issue(facility=facility, status='open')
login(make_user(role='admin'))
client.post(link_url(a),
data={'link_type': 'duplicate', 'linked_issue_id': str(b.id)},
follow_redirects=True)
_db.session.refresh(a)
_db.session.refresh(b)
assert a.status == 'open' and a.resolved_at is None and a.assigned_to is None
assert b.status == 'open' and b.resolved_at is None
@pytest.mark.parametrize('payload', [
{'link_type': 'related', 'linked_issue_id': 'abc'},
{'link_type': 'related', 'linked_issue_id': ''},
{'link_type': 'nonsense', 'linked_issue_id': '1'},
{'link_type': '', 'linked_issue_id': '1'},
{},
])
def test_malformed_link_requests_are_rejected_without_error(
client, login, make_user, make_facility, make_issue, link_url, payload):
facility = make_facility()
a = make_issue(facility=facility)
make_issue(facility=facility)
login(make_user(role='admin'))
res = client.post(link_url(a), data=payload, follow_redirects=True)
assert res.status_code == 200
assert IssueLink.query.count() == 0
def test_an_issue_cannot_be_linked_to_itself(client, login, make_user,
make_facility, make_issue, link_url):
facility = make_facility()
a = make_issue(facility=facility)
login(make_user(role='admin'))
client.post(link_url(a),
data={'link_type': 'related', 'linked_issue_id': str(a.id)},
follow_redirects=True)
assert IssueLink.query.count() == 0
def test_the_same_pair_cannot_be_linked_twice_in_either_direction(
client, login, make_user, make_facility, make_issue, link_url):
facility = make_facility()
a, b = make_issue(facility=facility), make_issue(facility=facility)
login(make_user(role='admin'))
client.post(link_url(a), data={'link_type': 'related',
'linked_issue_id': str(b.id)},
follow_redirects=True)
client.post(link_url(a), data={'link_type': 'duplicate',
'linked_issue_id': str(b.id)},
follow_redirects=True)
client.post(link_url(b), data={'link_type': 'duplicate',
'linked_issue_id': str(a.id)},
follow_redirects=True)
assert IssueLink.query.count() == 1
def test_unlinking_removes_the_row_and_works_from_either_end(
client, login, make_user, make_facility, make_issue, db):
facility = make_facility()
a, b = make_issue(facility=facility), make_issue(facility=facility)
db.session.add(IssueLink(issue_id=a.id, linked_issue_id=b.id,
link_type='related'))
db.session.commit()
link_id = IssueLink.query.one().id
login(make_user(role='admin'))
# from the far end, which is the row's linked_issue_id
client.post(f'/issues/{b.id}/links/{link_id}/delete', follow_redirects=True)
assert IssueLink.query.count() == 0
def test_cannot_delete_a_link_between_two_other_issues(
client, login, make_user, make_facility, make_issue, db):
"""The link id is posted by the client, so it must be checked against
the issue in the URL -- otherwise any link is deletable from anywhere."""
facility = make_facility()
a, b, unrelated = (make_issue(facility=facility) for _ in range(3))
db.session.add(IssueLink(issue_id=a.id, linked_issue_id=b.id,
link_type='related'))
db.session.commit()
link_id = IssueLink.query.one().id
login(make_user(role='admin'))
client.post(f'/issues/{unrelated.id}/links/{link_id}/delete',
follow_redirects=True)
assert IssueLink.query.count() == 1
# ── Permission ───────────────────────────────────────────────────────────────
@pytest.mark.parametrize('role', ['admin', 'director', 'auditor'])
def test_issue_managers_may_link(client, login, make_user, make_facility,
make_issue, link_url, role):
facility = make_facility()
a, b = make_issue(facility=facility), make_issue(facility=facility)
login(make_user(role=role))
client.post(link_url(a), data={'link_type': 'related',
'linked_issue_id': str(b.id)},
follow_redirects=True)
assert IssueLink.query.count() == 1
def test_the_assignee_may_link_their_own_issue(client, login, make_user, db,
make_facility, make_issue,
make_project, link_url):
project = make_project()
facility = make_facility(project=project)
inspector = make_user(role='inspector')
db.session.add(InspectorAssignment(user_id=inspector.id, project_id=project.id))
db.session.commit()
a = make_issue(facility=facility, assigned_to=inspector.id)
b = make_issue(facility=facility)
login(inspector)
client.post(link_url(a), data={'link_type': 'related',
'linked_issue_id': str(b.id)},
follow_redirects=True)
assert IssueLink.query.count() == 1
def test_a_project_manager_may_not_link(client, login, make_user,
make_facility, make_issue, link_url):
"""Matches the page's existing can_edit set. Widening this is a decision,
not an accident -- if it changes, issues/view.html must change too."""
facility = make_facility()
a, b = make_issue(facility=facility), make_issue(facility=facility)
login(make_user(role='project_manager'))
res = client.post(link_url(a), data={'link_type': 'related',
'linked_issue_id': str(b.id)})
assert res.status_code == 403
assert IssueLink.query.count() == 0
def test_a_customer_may_not_link_even_on_their_own_facility(
client, login, make_user, db, make_facility, make_issue,
make_project, link_url):
project = make_project()
facility = make_facility(project=project)
customer = make_user(role='customer')
db.session.add(CustomerAssignment(user_id=customer.id, project_id=project.id))
db.session.commit()
a, b = make_issue(facility=facility), make_issue(facility=facility)
login(customer)
res = client.post(link_url(a), data={'link_type': 'related',
'linked_issue_id': str(b.id)})
assert res.status_code == 403
assert IssueLink.query.count() == 0
# ── Scope: the part that actually matters ────────────────────────────────────
def test_cannot_link_to_an_issue_outside_your_scope(
client, login, make_user, db, make_facility, make_issue,
make_project, link_url):
"""An inspector must not be able to attach another contract's issue."""
mine = make_project('Mine')
theirs = make_project('Theirs')
my_facility = make_facility(project=mine)
their_facility = make_facility(project=theirs)
inspector = make_user(role='inspector')
db.session.add(InspectorAssignment(user_id=inspector.id, project_id=mine.id))
db.session.commit()
a = make_issue(facility=my_facility, assigned_to=inspector.id)
out_of_scope = make_issue(facility=their_facility,
description='another contract')
login(inspector)
res = client.post(link_url(a),
data={'link_type': 'related',
'linked_issue_id': str(out_of_scope.id)},
follow_redirects=True)
assert IssueLink.query.count() == 0
# And the refusal must not confirm the issue exists.
body = res.get_data(as_text=True)
assert 'another contract' not in body
def test_the_panel_hides_a_link_whose_far_end_is_out_of_scope(
client, login, make_user, db, make_facility, make_issue, make_project):
"""An admin can link across contracts. A customer at one end must still not
read the issue at the other."""
mine = make_project('Mine')
theirs = make_project('Theirs')
my_facility = make_facility(project=mine)
their_facility = make_facility(project=theirs)
customer = make_user(role='customer')
db.session.add(CustomerAssignment(user_id=customer.id, project_id=mine.id))
db.session.commit()
visible = make_issue(facility=my_facility, description='my own issue')
hidden = make_issue(facility=their_facility,
description='SECRET other customer issue')
db.session.add(IssueLink(issue_id=visible.id, linked_issue_id=hidden.id,
link_type='related'))
db.session.commit()
login(customer)
body = client.get(f'/issues/{visible.id}').get_data(as_text=True)
assert 'my own issue' in body
assert 'SECRET other customer issue' not in body
assert f'/issues/{hidden.id}' not in body
def test_link_search_is_scoped(client, login, make_user, db, make_facility,
make_issue, make_project):
mine = make_project('Mine')
theirs = make_project('Theirs')
my_facility = make_facility(project=mine)
their_facility = make_facility(project=theirs)
inspector = make_user(role='inspector')
db.session.add(InspectorAssignment(user_id=inspector.id, project_id=mine.id))
db.session.commit()
a = make_issue(facility=my_facility, description='mine mine mine')
findable = make_issue(facility=my_facility, description='findable mine')
hidden = make_issue(facility=their_facility, description='findable theirs')
login(inspector)
data = client.get(f'/issues/{a.id}/link-search?q=findable').get_json()
ids = {r['id'] for r in data['results']}
assert findable.id in ids
assert hidden.id not in ids
def test_link_search_excludes_self_and_already_linked(
client, login, make_user, db, make_facility, make_issue):
facility = make_facility()
a = make_issue(facility=facility, description='widget one')
already = make_issue(facility=facility, description='widget two')
free = make_issue(facility=facility, description='widget three')
db.session.add(IssueLink(issue_id=a.id, linked_issue_id=already.id,
link_type='related'))
db.session.commit()
login(make_user(role='admin'))
data = client.get(f'/issues/{a.id}/link-search?q=widget').get_json()
ids = {r['id'] for r in data['results']}
assert ids == {free.id}
def test_link_search_finds_by_issue_number(client, login, make_user,
make_facility, make_issue):
facility = make_facility()
a = make_issue(facility=facility)
target = make_issue(facility=facility, description='nothing in common')
login(make_user(role='admin'))
data = client.get(f'/issues/{a.id}/link-search?q={target.id}').get_json()
assert target.id in {r['id'] for r in data['results']}
def test_link_search_returns_nothing_for_an_unscoped_inspector(
client, login, make_user, make_facility, make_issue):
"""Strict scoping: no assignments means no candidates, not all of them."""
facility = make_facility()
a = make_issue(facility=facility, description='findable')
make_issue(facility=facility, description='findable too')
inspector = make_user(role='inspector')
login(inspector)
# No InspectorAssignment -> cannot even open the issue.
assert client.get(f'/issues/{a.id}/link-search?q=findable').status_code == 403
# ── Rendering ────────────────────────────────────────────────────────────────
def test_the_panel_renders_both_directions(client, login, make_user, db,
make_facility, make_issue):
facility = make_facility()
subject = make_issue(facility=facility, description='the one being viewed')
original = make_issue(facility=facility, description='the original')
other = make_issue(facility=facility, description='a related thing')
db.session.add(IssueLink(issue_id=subject.id, linked_issue_id=original.id,
link_type='duplicate'))
db.session.add(IssueLink(issue_id=other.id, linked_issue_id=subject.id,
link_type='duplicate'))
db.session.commit()
login(make_user(role='admin'))
body = client.get(f'/issues/{subject.id}').get_data(as_text=True)
assert 'Duplicate of' in body # subject -> original
assert 'Duplicated by' in body # other -> subject
assert f'/issues/{original.id}' in body
assert f'/issues/{other.id}' in body
def test_the_panel_renders_with_no_links(client, login, make_user,
make_facility, make_issue):
facility = make_facility()
issue = make_issue(facility=facility)
login(make_user(role='admin'))
body = client.get(f'/issues/{issue.id}').get_data(as_text=True)
assert 'Linked Issues' in body
assert 'No linked issues' in body
+72
View File
@@ -167,6 +167,78 @@ def test_issues_list_scopes_a_customer_to_their_own_facilities(
assert 'other customer only' not in body assert 'other customer only' not in body
# ── Issue detail scope gate ──────────────────────────────────────────────────
# issues.view() had two inline scope blocks; they were collapsed into
# _issue_readable_by() so the linked-issues panel could reuse the same rule.
# These pin the behaviour that gate must keep.
def test_issue_view_allows_an_inspector_inside_their_contract(
client, login, make_user, db, make_facility, make_issue, project):
inspector = make_user(role='inspector')
db.session.add(InspectorAssignment(user_id=inspector.id,
project_id=project.id))
db.session.commit()
issue = make_issue(facility=make_facility(project=project))
login(inspector)
assert client.get(f'/issues/{issue.id}').status_code == 200
def test_issue_view_denies_an_inspector_outside_their_contract(
client, login, make_user, db, make_facility, make_issue,
project, other_project):
inspector = make_user(role='inspector')
db.session.add(InspectorAssignment(user_id=inspector.id,
project_id=project.id))
db.session.commit()
issue = make_issue(facility=make_facility(project=other_project),
description='not yours')
login(inspector)
res = client.get(f'/issues/{issue.id}', follow_redirects=True)
assert 'not yours' not in res.get_data(as_text=True)
def test_issue_view_denies_an_inspector_with_no_assignments(
client, login, make_user, make_facility, make_issue, project):
"""Strict scoping: no assignments means no access, not full access."""
issue = make_issue(facility=make_facility(project=project),
description='strictly scoped')
login(make_user(role='inspector'))
res = client.get(f'/issues/{issue.id}', follow_redirects=True)
assert 'strictly scoped' not in res.get_data(as_text=True)
def test_issue_view_denies_a_customer_outside_their_contract(
client, login, make_user, db, make_facility, make_issue,
project, other_project):
customer = make_user(role='customer')
db.session.add(CustomerAssignment(user_id=customer.id,
project_id=project.id))
db.session.commit()
issue = make_issue(facility=make_facility(project=other_project),
description='other customer only')
login(customer)
res = client.get(f'/issues/{issue.id}', follow_redirects=True)
assert 'other customer only' not in res.get_data(as_text=True)
def test_issue_view_allows_a_customer_inside_their_contract(
client, login, make_user, db, make_facility, make_issue, project):
customer = make_user(role='customer')
db.session.add(CustomerAssignment(user_id=customer.id,
project_id=project.id))
db.session.commit()
issue = make_issue(facility=make_facility(project=project),
description='mine to read')
login(customer)
res = client.get(f'/issues/{issue.id}')
assert res.status_code == 200
assert 'mine to read' in res.get_data(as_text=True)
# ── Reports overview ───────────────────────────────────────────────────────── # ── Reports overview ─────────────────────────────────────────────────────────
def test_reports_overview_renders_for_admin(client, login, make_user, def test_reports_overview_renders_for_admin(client, login, make_user,