Fix unique milestone names perms

[jasquat]

This fixes unique milestone api call for basic permissions.

[coderabbitai]

Warning

Review limit reached

@jasquat, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 minute and 8 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ffb32b76-bb3a-49b3-a102-4f3c304646d1

📥 Commits

Reviewing files that changed from the base of the PR and between f4e0f36 and e7b5da2.

📒 Files selected for processing (1)

• spiffworkflow-frontend/src/components/ProcessInstanceListTableWithFilters.tsx

📝 Walkthrough

Walkthrough

This PR extends the unique milestone names endpoint with optional filtering by user relation and process model identifier. The backend adds service layer methods to filter instances by user involvement and compute distinct milestone names. The frontend provides helper utilities to construct query-parameterized URIs and merge selected values into option lists, then integrates them into the filter component with permission-gated fetching.

Changes

Unique Milestone Names Filtering

Layer / File(s)	Summary
API Contract & Service Layer spiffworkflow-backend/src/spiffworkflow_backend/api.yml, spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py	GET /process-instances/unique-milestone-names accepts optional with_relation_to_me and process_model_identifier query parameters. New ProcessInstanceReportService methods filter instances by user relation via human task linkage and compute sorted, distinct milestone names from filtered queries.
Authorization & Controller Implementation spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py, spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py	Authorization service grants read permission on the endpoint. Controller accepts parameters, constructs filter list, determines user scoping based on permissions, and delegates milestone computation to the service. Existing run_process_instance_report is refactored to use the new filter_by_with_relation_to_me helper.
Backend Tests spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.py, spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py	Integration tests validate endpoint scoping to the requesting user's relation, global-access user override, privileged-user scoping via query parameter, and process-model filtering. Unit test confirms the new permission is registered in basic permissions.
Frontend Helper Functions spiffworkflow-frontend/src/helpers.tsx, spiffworkflow-frontend/src/helpers.test.tsx	New buildUniqueMilestoneNamesPath constructs URIs with conditional query parameters; mergeSelectedStringOption ensures selected milestones appear at the front of option lists. Comprehensive unit tests verify both helpers across parameter combinations.
Frontend Component & Permission Integration spiffworkflow-frontend/src/hooks/UriListForPermissions.tsx, spiffworkflow-frontend/src/components/ProcessInstanceListTableWithFilters.tsx	UriListForPermissions registers the endpoint URI. ProcessInstanceListTableWithFilters requests permission, derives process model identifier from report metadata, conditionally fetches milestones only when filters are enabled and permission is granted, and renders filter controls using merged option lists to preserve selected milestones even when not returned by the backend.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• sartography/spiff-arena#1805: Related change to useUriListForPermissions extending the targetUris map.

Suggested reviewers

• burnettk

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Fix unique milestone names perms' is abbreviation-heavy and vague, using 'perms' instead of 'permissions', making it unclear whether this addresses permissions bugs, missing permissions, or permission scope issues.	Use a more descriptive title like 'Add permission scoping to unique milestone names endpoint' or 'Fix permissions for unique milestone names API' to clarify the specific change.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description states the PR 'fixes unique milestone api call for basic permissions', which directly relates to the changeset that adds permission checks, authorization scoping, and permission assignments to the unique milestone names endpoint.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-unique-milestone-names-perms

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py (1)

687-704: ⚡ Quick win

Drop selectinload(...) for with_entities(...) queries
get_basic_query()’s selectinload(ProcessInstanceModel.process_initiator) doesn’t generally cause a runtime error when unique_milestone_names() narrows the query to a scalar column via with_entities(ProcessInstanceModel.last_milestone_bpmn_name). However, the eager-load option can’t be applied meaningfully once the result is no longer ORM entities, so the option is effectively a no-op. Consider disabling/omitting eager-loads for this specific path for clarity/efficiency.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py`
around lines 687 - 704, The query returned by cls.get_basic_query(filters) still
carries the selectinload(ProcessInstanceModel.process_initiator) option which
becomes a no-op for scalar queries; before calling
.with_entities(ProcessInstanceModel.last_milestone_bpmn_name) in
unique_milestone_names (or the method containing this snippet), explicitly
disable eager loads on process_instance_query (e.g. call process_instance_query
= process_instance_query.enable_eagerloads(False) or otherwise strip/avoid the
selectinload option) so the query no longer retains the selectinload and the
resulting scalar query is clear and efficient.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py`:
- Around line 663-669: The logic that sets should_scope_to_requesting_user
wrongfully treats users with model-scoped permissions as basic users; update the
check in the block using should_scope_to_requesting_user, with_relation_to_me,
and AuthorizationService.user_has_permission so that if process_model_identifier
is present you first ask AuthorizationService.user_has_permission(user=g.user,
permission="read",
target_uri=f"/process-instances/{process_model_identifier}/*") (or equivalent
model-scoped target) and only fall back to the global "/process-instances" check
when no model identifier is given or model-scoped read is denied; keep the
existing append to filters for with_relation_to_me when the combined result
requires scoping.

In
`@spiffworkflow-frontend/src/components/ProcessInstanceListTableWithFilters.tsx`:
- Around line 513-537: The effect that fetches milestone names (useEffect) can
have overlapping responses overwrite state; modify the
HttpService.makeCallToBackend call in this effect to ignore stale responses by
attaching a request-scoped guard (e.g. a locally generated requestId or capture
of uniqueMilestoneNamesPath) and only calling setLastMilestones if the guard
still matches current scope when the successCallback runs; implement the guard
inside the successCallback (or via AbortController if supported) and reference
useEffect, HttpService.makeCallToBackend, uniqueMilestoneNamesPath, and
setLastMilestones to locate the change.

---

Nitpick comments:
In
`@spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py`:
- Around line 687-704: The query returned by cls.get_basic_query(filters) still
carries the selectinload(ProcessInstanceModel.process_initiator) option which
becomes a no-op for scalar queries; before calling
.with_entities(ProcessInstanceModel.last_milestone_bpmn_name) in
unique_milestone_names (or the method containing this snippet), explicitly
disable eager loads on process_instance_query (e.g. call process_instance_query
= process_instance_query.enable_eagerloads(False) or otherwise strip/avoid the
selectinload option) so the query no longer retains the selectinload and the
resulting scalar query is clear and efficient.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fa238093-1cac-41a1-98e2-08988f811547

📥 Commits

Reviewing files that changed from the base of the PR and between 50eb4a0 and f51ae74.

📒 Files selected for processing (10)

• spiffworkflow-backend/src/spiffworkflow_backend/api.yml
• spiffworkflow-backend/src/spiffworkflow_backend/routes/process_instances_controller.py
• spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py
• spiffworkflow-backend/src/spiffworkflow_backend/services/process_instance_report_service.py
• spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_instances_controller.py
• spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_authorization_service.py
• spiffworkflow-frontend/src/components/ProcessInstanceListTableWithFilters.tsx
• spiffworkflow-frontend/src/helpers.test.tsx
• spiffworkflow-frontend/src/helpers.tsx
• spiffworkflow-frontend/src/hooks/UriListForPermissions.tsx