Follow-up from "Feat: Resolve "Ability to cancel a flow session" backend changes"
The following discussions from !215795 (merged) should be addressed:
-
@GitLabDuo started a discussion: (+2 comments)
The error message "You do not have permission to cancel this session." is misleading when used for non-cancellation operations like
finish,pause, orresume. Consider using a more generic message like "You do not have permission to update this session." or make the message dynamic based on the status event. -
@.luke started a discussion:
Suggestion (not blocking)
It looks like this is already gated before
#handle_status_eventis called. It seems redundant, can we remove this? In the tests we simulate a user first having permission and then on the next check having the permission be false, but I don't think that's a likely scenario and it's likely the permission is cached anyway at this point as theuserspecific conditions are probably all scoped byuser. -
@.luke started a discussion:
Suggestion (not blocking)
This looks like a duplicate of
it "skips non-cancelable pipelines" -
@.luke started a discussion:
Suggestion (not blocking)
It would be good to include some pipelines that should not be scoped by the scope (if they're not already being created through the factory already?), just to prove that we're not over selecting:
- A workload and pipeline that is not associated with the workflow
- A pipeline that is not associated with a workload at all
let(:pipeline4) { create(:ci_pipeline, project: project) } let(:pipeline5) { create(:ci_pipeline, project: project) } let(:workload1) { create(:ci_workload, pipeline: pipeline1, project: project) } let(:workload2) { create(:ci_workload, pipeline: pipeline2, project: project) } let(:workload3) { create(:ci_workload, pipeline: pipeline3, project: project) } let(:workload4) { create(:ci_workload, pipeline: pipeline4, project: project) } -
@.luke started a discussion:
Suggestion (not blocking)
I'm not sure how many workloads a workflow might be (it may be very few) but if there is a possibility of quite a number, we might want to look at whether we can optimise this through SQL.