-
-
Notifications
You must be signed in to change notification settings - Fork 295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added autocomplete input field for assigning bulk default user #3250
base: master
Are you sure you want to change the base?
Conversation
6b9c235
to
5dfe4be
Compare
Testplan.mp4 |
@atodorov did you get time to look into this PoC? |
Sorry, not yet. I will try to look into it by the end of this week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally looks good but needs changes in order for us to be able to use this widget elsewhere and for the user experience to be better.
return element.username | ||
}, | ||
source: function (query, processSync, processAsync) { | ||
jsonRPC('User.filter', { username__icontains: query }, function (data) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: User.filter
depends on having the auth.view_user
permission which currently silently prints errors in the console if the permission is not granted.
I'm not sure if/how we want to inform users that their auto-complete searches may not work because they are missing the permission.
Maybe we should default to the old implementation if permission isn't granted and allow auto-complete only when it is.
Thank you for your feedback @atodorov! I will make those changes ASAP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better, some changes needed to address issues with the current implementation.
<button type="button" class="close" data-dismiss="modal" aria-hidden="true" aria-label="Close"> | ||
<span class="pficon pficon-close"></span> | ||
</button> | ||
<h4 class="modal-title" id="default-tester-modal-title">{% trans "Default Tester" %}</h4> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default Tester
-> Default tester
to minimize variations in translation strings
<h4 class="modal-title" id="default-tester-modal-title">{% trans "Default Tester" %}</h4> | |
<h4 class="modal-title" id="default-tester-modal-title">{% trans "Default tester" %}</h4> |
@@ -22,6 +22,7 @@ <h1 class="col-md-12" style="margin-top: 0"> | |||
data-perm-add-testcase="{{ perms.testcases.add_testcase }}" | |||
data-perm-add-comment="{{ perms.django_comments.add_comment }}" | |||
data-perm-delete-comment="{{ perms.django_comments.delete_comment }}" | |||
data-perm-view-user="{{ perms.auth.view_user }}" | |||
data-trans-username-email-prompt="{% trans 'Enter username, email or user ID:' %}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused, needs to be removed:
data-trans-username-email-prompt="{% trans 'Enter username, email or user ID:' %}" |
}) | ||
} | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few issues with how this function/modal behaves:
Issue #1
- Use it once to assign a default tester. I chose "alice" then
- Mark some test cases and trigger the default tester modal again -> the text "alice" is still present inside the input field, it should be empty field instead
Issue #2
Every time the modal opens the typeahead handler gets initialized again. Using the modal 5-6 times without reloading the page leads to a chain of User.filter
RPC calls instead of having only 1 call. This visibly slows down the page. This is similar to #3281 .
Closes #2510