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

Alert whenever a @mention is of someone not in the room #603

Open
jayvdb opened this issue Aug 5, 2018 · 10 comments · May be fixed by #630
Open

Alert whenever a @mention is of someone not in the room #603

jayvdb opened this issue Aug 5, 2018 · 10 comments · May be fixed by #630

Comments

@jayvdb
Copy link
Member

jayvdb commented Aug 5, 2018

Similar to #317 , but for room membership.

The wording needs to be moderately toned, as it isnt 'bad' to mention a org member who isnt in the room - on gitter, they will get a notification to join the room, and they can read the room without joining, so it is OK to ping them, and they can ignore it.

Also c.f. #602

@jayvdb
Copy link
Member Author

jayvdb commented Aug 5, 2018

However this also has a security aspect to it, as it ensure there is checking happening on any username mentioned, which can prevent mistakes in process due to incorrect username being used, and allow mistakes to be fixed quickly.

@abhishalya
Copy link
Member

@jayvdb Please assign this to me 😃

@abhishalya
Copy link
Member

@jayvdb Has this been already taken care of? I found this:

if not self.is_room_member(invitee, msg):
    yield '@{} is not a member of this room.'.format(invitee)
    return

@jayvdb
Copy link
Member Author

jayvdb commented Oct 13, 2018

That is a specific case, operational only in one command.

We want a generic solution which works all the time to replace those lines ;-)

See errbot filters.

@abhishalya
Copy link
Member

abhishalya commented Oct 15, 2018

@jayvdb So if I understand correctly if within any message if @mention is there and the person is not from the room then that should produce a warning message.

I hope this can be solved by checking if there is any mention and if there is, check for room_member and then pass the warning message accordingly. I hope we have to use @cmdfiler for this.

Let me know if I'm wrong 😅

@jayvdb
Copy link
Member Author

jayvdb commented Oct 15, 2018

That sounds roughly correct.

@abhishalya
Copy link
Member

errbot already has a callback_mention method which is triggered whenever someone is mentioned, the triggering is not currently implemented in gitter backend but it can be done like in many other backends.

Reference: errbotio/errbot#550

I will here create a cmdfilter which would do the job we want. But if needed, it can be improved.

@abhishalya
Copy link
Member

@jayvdb This is my solution:

  1. We can append a small code to the callback_message function since the backend already has it. Later we can create a separate callback_mention method for this.
  2. We can get the person mentioned through the message body but then its identifier is required to check if the mentioned person is in the room or not. This requires the build_identifier function to work properly. Currently the function is incorrectly implemented and hence requires changes to that first.

I think this issue depends on following:

  1. Fixing build_identifier.
  2. Implementing callback_mention.

Should I create both issues on the backend. I'll assign them to myself. Please confirm them first 😅

@Vamshi99
Copy link
Member

We can append a small code to the callback_message function since the backend already has it. Later we can create a separate callback_mention method for this.

@abhishalya There is no need to create a new callback_mention func, just use callback_message like here and check if the users mentioned in the message is member of that room or not using is_room_member.

We can get the person mentioned through the message body but then its identifier is required to check if the mentioned person is in the room or not

There is no need of build_identifier func here, why can't you use this

@abhishalya
Copy link
Member

abhishalya commented Oct 18, 2018

@Vamshi99

There is no need to create a new callback_mention func, just use callback_message like here and check if the users mentioned in the message is member of that room

Yes that is my current idea. But, rather than doing this we can simply use the callback_mention which would be automatically triggered once someone is mentioned. Its just an improvement as callback_mention is there in errbot and is used in most backends.

There is no need of build_identifier func here, why can't you use this

It only works for a single function and as @jayvdb said we need a more generic solution which would work for all.

abhishalya added a commit to abhishalya/corobo that referenced this issue Oct 23, 2018
The fix introduces removal of is_room_member
function and instead uses callback_message to warn
whenever `@mention` is not in room.

Closes coala#603
@abhishalya abhishalya linked a pull request Oct 23, 2018 that will close this issue
6 tasks
abhishalya added a commit to abhishalya/corobo that referenced this issue Mar 8, 2019
The fix introduces removal of is_room_member
function and instead uses callback_message to warn
whenever `@mention` is not in room.

Closes coala#603
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants