The question here is if there is any way the backend can automatically filter out placeholder users on the API / GraphQL level so we don't have to go fix an unknown number of dropdowns (I don't think I have the full context to update all the user dropdowns in issues, MRs, epics...).
I'm afraid to remove the author from the assignees' list; it needs to be a frontend change.
The frontend automatically adds the author as a suggested user. The frontend uses GraphQL 1GraphQL 2 to fetch the assignees including the author, then adds the author as a suggested user. So, the backend doesn't control that.
For the mentions, it's a backend change. We can hide the placeholder users by making the following changes (Note: I didn't test thoroughly):
diff --git a/app/finders/autocomplete/users_finder.rb b/app/finders/autocomplete/users_finder.rbindex fc45eb6db5fb..2f1752e3b8d8 100644--- a/app/finders/autocomplete/users_finder.rb+++ b/app/finders/autocomplete/users_finder.rb@@ -34,7 +34,7 @@ def execute # Include current user if available to filter by "Me" items.unshift(current_user) if prepend_current_user?- items.unshift(author) if prepend_author? && author&.active?+ items.unshift(author) if prepend_author? && author&.active? && !author&.import_user? && !author&.placeholder? end items = filter_users_by_push_ability(items)diff --git a/app/services/concerns/users/participable_service.rb b/app/services/concerns/users/participable_service.rbindex e18b7f5923f7..a8e7d238414f 100644--- a/app/services/concerns/users/participable_service.rb+++ b/app/services/concerns/users/participable_service.rb@@ -15,6 +15,7 @@ module ParticipableService def noteable_owner return [] unless noteable && noteable.author.present?+ return [] if noteable.author.placeholder? || noteable.author.import_user? [noteable.author].tap do |users| preload_status(users)@@ -25,6 +26,7 @@ def participants_in_noteable return [] unless noteable users = noteable.participants(current_user)+ users = users.reject { |user| user.placeholder? || user.import_user? } sorted(users) end
The frontend automatically adds the author as a suggested user. The frontend uses GraphQL 1GraphQL 2 to fetch the assignees including the author, then adds the author as a suggested user. So, the backend doesn't control that.
Can we modify the graphql queries for getMrAssignees and issueAssignees by adding an optional (default=false) query param includePlaceholders?
@justin_ho For the backend segment, from the code Rodrigo linked to above, it looks like you need some new properties for issuableAuthor. Could you confirm the backend could expose a property in the GraphQL UserCore object.
Martin mentioned in #481925 (comment 2091411116) that there are multiple frontend components. Do you know yet if they use the same data source (for example the GraphQL UserCore type) or would there be multiple places on the backend that we would need to expose a property?
In terms of just the GraphQL UserCore type, it has some booleans fields human and bot. If we followed that pattern we would add 2 new fields placeholder and import_user. But I think it might be a better idea to add one just one field type, which would be an enum of user types. Otherwise, each field the frontend asks for adds an extra 1 to the query complexity score, so it might be better to just have one field that can be used in future?
For example:
{project(fullPath:"gitlab-org/gitlab"){issue(iid:"481925"){author{type# => You would test for `PLACEHOLDER` or `IMPORT_USER`}}}}
@justin_ho I've made this a weight 2 issue, with backend-weight1 and frontend-weight1. I'm unsure of the frontend weight because it's mentioned there are multiple components, so if the frontend weight should increase because please bump the weight up
I think this issue needs a bit more research on my end but since it's scheduled for next milestone, I'll have a look closer towards the end of this milestone. Tentatively setting this to frontend-weight2 as it definitely won't be straightforward to work with code that is not our domain.
@justin_ho Did you get to investigate this further? This is scheduled for the current milestone but currently at risk of slipping. Would be good if we could have a solid understanding of what needs to be done both on the backend and frontend before we move it over to %17.7
Hi @rodrigo.tomonari , @.luke I'm picking this up. I want to confirm what part of this issue is outstanding on the backend, as I see that @wortschimerged an MR for the @mentions part. Is my understanding correct that that merged change is not an adequate BE fix for this 'placeholder user being assigned' bug so-to-speak? Instead I should look into this comment?
@obaiye Yes, that's my understanding too. The @mention bug has been fixed, but not the placeholder user appearing in assign dropdowns. The problem for that is what Rodrigo linked to #481925 (comment 2082539343).
The new type field could return a GraphQL enum string of the type of the user. When the FE is handling the author, it could test if type were "PLACEHOLDER" or "IMPORT_USER" (two possible enum values) and if so, skip the logic of adding them to the select list as options.
This is the problem in a screen capture, because the author of an issue is a placholder user (called Placeholder lukes) they also appear in the assign dropdown.
If it's helpful to sync in a zoom chat let me know!
In the widget (sidebar_assignees_widget.vue) that houses the Assignees Dropdowns there are two separate queries called within its hierarchy, eg. for Issues, it would be 1) issueAssignees and workspaceAutocompleteUsersSearch. The merge request feature also uses this widget and has its own equivalent queries, one of which is projectAutocompleteUsersSearchWithMRPermissions.
The autocomplete queries are responsible for populating the full dropdown lists in the respective instances of the sidebar. From what I can tell, filtering out in the query files users with type==placeholder is either not possible with graphql or fairly complicated. But also, these autocomplete queries are used in many different features, including WorkItems, etc.
So I got the suggestion, and wondered what you'd think of this - to instead pass an optional 'excludeUserTypes' argument to the autocomplete graphql resolver so that the backend itself excludes the users, instead of handling that filtering on the frontend side. Something like
class AutocompleteUsersResolver < BaseResolver type [::Types::Users::AutocompletedUserType], null: true # other stuff argument :exclude_user_types, [GraphQL::Types::String], required: false, description: 'User types to exclude from the results.' def resolve(search: nil, exclude_user_types: []) ::Autocomplete::UsersFinder.new( current_user: context[:current_user], project: project, group: group, params: { search: search, exclude_user_types: exclude_user_types } ).execute end
I'd be modifying the users finder as well. The advantage of this would be I would only pass an excludeUserTypes param from the SidebarAssigneeWidget or its child UserSelect, and thus (hopefully) not cause unexpected issues in other parts of the app that rely on the autocompleteUsers endpoint?
@obaiye Do you see placeholders coming back from that autocomplete search? The finder scopes users by .non_internal, the scope is defined here and so should only return users that are one of these user types. Because placeholder and import users have a user type of placeholder and import_user they should already be being scoped out of the autocomplete search.
@.luke Actually, you are right, the autocomplete returns one list and it seems the placeholder user is then added in on the frontend. I'm just trying to figure out how. Interesting.
@.luke I had a conversation with @justin_ho earlier today and we were wondering if the behavior you observed in this issue happens before or after the reassignment process? From the steps it looks like this issue happens before a placeholder user is being mapped to an actual user. If this is the case, I wonder if the existing behavior is intended behavior as we always want the assigned user to appear at the top of the list.
@wortschi@justin_ho Yes, this happens before the reassignment process. We have some logic that puts the author into these suggestion list by default. It happens when there is no assignee too. If you select a placeholder user as the assignee the save fails, and it isn't assigned.
@.luke Thank you for the additional details and the video.
One last question to make sure we're on the same page: In the assignee dropdown, there's only one placeholder user available (i.e., the issue's author). Other placeholder users are not available to select as they are filtered out already. Correct?
@m_frankiewicz@.luke@justin_ho It appears that only one placeholder user (i.e., the author) is available in the dropdowns. Other placeholder (besides the author) users are not available in the dropdown. Also, selecting the placeholder user as assignee or reviewer is not possible via the UI.
We still need to refine this as there are user dropdowns in multiple places and they don't share the same component. All that being said, I'm moving this to the post release epic as this can be considered as a UX bug.
only one placeholder user (i.e., the author) is available in the dropdowns. Other placeholder (besides the author) users are not available in the dropdown. Also, selecting the placeholder user as assignee or reviewer is not possible via the UI.
this is not a big deal and we can definitely wait with the fix.
We still need to refine this as there are user dropdowns in multiple places and they don't share the same component.
All bugux issues need to have a proper severity label set.
Please add a severity label, remove the automation:ux-missing-labels label, and then reply to this comment briefly explaining your reasoning for providing this severity.
If you are not the DRI for this area and would like help determining the best severity, please @ the appropriate person for assistance.
Martin Wortschackchanged title from Bug: Placeholder user appears in assign and @-mention dropdowns to Bug: Placeholder user appears in assign dropdowns
changed title from Bug: Placeholder user appears in assign and @-mention dropdowns to Bug: Placeholder user appears in assign dropdowns
Martin Wortschackchanged the descriptionCompare with previous version