Add filter for SignonUsers to AdminUI#206
Conversation
davidgisbey
force-pushed
the
2449-add-api-user-details-to-admin-ui
branch
from
9aeec18 to
6fa7fae
Compare
davidgisbey
force-pushed
the
2449-add-api-user-details-to-admin-ui
branch
from
6fa7fae to
90fba60
Compare
davidgisbey
force-pushed
the
2449-add-api-user-details-to-admin-ui
branch
from
90fba60 to
73006b5
Compare
davidgisbey
force-pushed
the
2449-add-api-user-details-to-admin-ui
branch
from
73006b5 to
cf640fb
Compare
kevindew
left a comment
kevindew
left a comment
There was a problem hiding this comment.
Nice work. I'm a bit confused about the conversation_id select field, so happy to chat over that. Also I think we may need to flag this more specifically as an API user or consider changing the web code a bit for consistency with signon_user storage.
| signon_user = conversation.signon_user | ||
| if signon_user.present? | ||
| rows << { | ||
| field: "Signon user", |
There was a problem hiding this comment.
Shall we name this API user to reflect our expectation that this is used for API?
(aside: I hadn't thought about it before, but we do actually have a slightly odd situation with reference to signon user where you need signon access to use the web chat one but we're not storing that it was done by a signon user)
There was a problem hiding this comment.
Yea it's an awkward one, because for now that's true but it's unlikely to be the case going were we to go live. I like the idea of naming this API user in the UI at least so there's a clear distinction. I guess an option would be to update the association to be
belongs_to :api_user, optional: true, class_name: "SignonUser", foreign_key: :signon_user_id
but it still feels a little awkward. Would def be less ambiguous though.
There was a problem hiding this comment.
Yeah I agree it feels a bit awkward - particularly with not knowing quite how the web version will iterate.
I think we'll leave the association as signon user for now, but we'll add an extra conditional for this showing and filtering of admin user that it also has an api source? how does that sound?
There was a problem hiding this comment.
yea sounds good to me. I'll merge this and apply those changes to the other PR then 👍
I think name is actually going to be more informative to us here provided the user has given it a reasonable name on creation of the API user on Signon. If we use the email it will likely end up being a devs email address which is less useful than something like "GOV.UK App team API user".
This pretty much copy and pastes the existing code for EarlyAccessUser. If an id is passed in it uses that to modify the scope to only include conversations associated with that signon user. I've chosen not to filter by signon user if an early access user id is passed in via user_id, as both filters are mutually exclusive.
This updates the QuestionsFilter to include a signon_user method. This will be used in the UI, much like the user and conversation methods.
davidgisbey
force-pushed
the
2449-add-api-user-details-to-admin-ui
branch
from
cf640fb to
e0af480
Compare
davidgisbey
force-pushed
the
2449-add-api-user-details-to-admin-ui
branch
from
e0af480 to
18ddcca
Compare
This adds the SignonUser details to the questions page when a QuestionsFilter#signon_user is present and QuestionsFilter#user is not. It also scopes the conversation filter to the SignonUsers conversations when one is present. I've added a new system spec for the filters.
When a question belongs to a conversation that has an assoiciated SignonUser, we should display the SignonUser details on the question. We also want to link to the questions show page with the SignonUsers id as a filter param like we do for EarlyAccessUsers.
davidgisbey
force-pushed
the
2449-add-api-user-details-to-admin-ui
branch
from
18ddcca to
2e77b97
Compare
3 participants
Description
This PR adds a filter for signon users to the questions show page and exposes the signon user details on the question show page.
It follows roughly the same pattern implemented for EarlyAccessUser in that you can only access the filter by clicking the link to
View all questionsfor the user on the question show page. If thesignon_user_idparams isn't present then no filter/user info will be rendered.I've made a few choices that might be worth discussing:
Screenshots
Show question page
Question index page
Filtered by conversation
Out of scope
The Trello card mentions adding a source filter so we can filter on
:apiorwebbased on if the convo was created by the API or not. That will be added in a follow up PR.Trello card
https://trello.com/b/BoPNvU55/stand-up-board-ai-govuk-llm-team-user-experience-govuk