Skip to content
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

User Context Tweaks #282

Merged
merged 3 commits into from
Mar 20, 2024
Merged

Conversation

andymandias
Copy link
Contributor

Two minor tweaks related to the new user context functionality:

  1. Only show the operator actions when the user is an operator.
  2. The context menu for usernames in the history view would always show the user with default access levels. As far as I can tell, that's because the user from message::Source::User(user) is being constructed from proto::User, which always has the default access levels. I didn't see a simple way to add the user lookup to message::Source::User construction, so I added it to the channel view.

I can split these up if one is desirable and the other is not.

@tarkah
Copy link
Member

tarkah commented Mar 16, 2024

The message::Source::User should be resolved with attributes. We do this in data::message when constructing it.

The reason you might see attributes missing is because this uses the attributes from our client state at the time we receive the message. If you first load Halloy and get a backlog of messages from your bouncer, these probably come in before we resolve WHO messages to update client state with attributes.

@casperstorm and I are aware of this potential gap. The upside of doing it this way however is old messages more accurately reflect permissions at the time the message was sent (ignoring the initial backlog timing issue).

We need to align on if we want to address this and maybe use some type safety to ensure we're always working with the desired user type.

@andymandias
Copy link
Contributor Author

Ahh, my apologies you're absolutely correct. I was testing with messages sent before attribute levels were changed. And, as you mention, the way I implemented this will reflect an inaccurate message history wrt attributes. Maybe the op context menu items could be only present in the nick_list? Regardless, I'll remove that part of this PR.

Comment on lines 37 to 40
let users = clients.get_channel_users(&state.server, &state.channel);
let our_user = users
.iter()
.find(|user| user.nickname() == our_nick.unwrap());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a clients.resolve_attributes you can use

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up!

@casperstorm
Copy link
Member

Can you add a entry to CHANGELOG as well?

@andymandias
Copy link
Contributor Author

I promise one of these days I'll remember to update changelog 😅

@casperstorm
Copy link
Member

Rebased on master. Tested and works great.

@casperstorm casperstorm merged commit 2f1f82e into squidowl:main Mar 20, 2024
1 check passed
@andymandias andymandias deleted the user-context-tweaks branch March 20, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants