Follow-up from "Backport of Multiple Assignees feature"
The following discussions from !11089 (merged) should be addressed:
-
@smcgivern started a discussion: We should split this and
assignee_id
by issue / MR, so that we don't acceptassignee_ids
for MRs. -
@smcgivern started a discussion: case selected_users.length
-
@smcgivern started a discussion: Same as above, we should only permit by issue / MR.
-
@smcgivern started a discussion: This should probably say 'removed assignee' in CE, but it's really not very important.
-
@smcgivern started a discussion: - issue.assignees.take(max).each do |assignee| # ...
-
@smcgivern started a discussion: We should probably move these blocks to their own
_sidebar/_assignee
partial -
@smcgivern started a discussion: This check if it's an MR is redundant now
😉 -
@smcgivern started a discussion: We could do these in a single hash literal, then merge with
options[:data]
. -
@smcgivern started a discussion: (+1 comment) Is this an array or an AR relation here? If it's a relation, we will perform an extra query for no reason. Try
issue.assignees.to_a.any?
. -
@smcgivern started a discussion: This should also be split into two partials.
-
@smcgivern started a discussion: We should make this clearer for CE, because in CE there can only be zero or one assignee.
-
@smcgivern started a discussion: a user -> the users?
-
@smcgivern started a discussion: We could also use
params.tap
here? -
@smcgivern started a discussion: (+1 comment) Nice!
-
@smcgivern started a discussion: user -> user