[WIP] Notify when a message with alert word is received

[Subhasish-Behera]

model/test_model/conftest : Notify when a message with alert word is received

What does this PR do?

Partial fix for #588

Tested?

• Manually
• Existing tests (adapted, if necessary)
• New tests added (for any new behavior)
• Passed linting & tests (each commit)

Commit flow

Notes & Questions

->No changes have been made to for private messages(not required as alert_words would be a subset case for this)
and in stream added the condition not is_muted_topic and not is_muted_stream when a message has flag:'has_alert_word'
-> Added tests specifically for stream with 3 parameters visual_notification_status,is_stream_muted,is_topic_muted
-> Added a new stream_msg_fixture in conftest.py as msg_fixture was present for private messages but not for stream. And the same fixture has been used in added tests.

[neiljp]

@Subhasish-Behera Thanks for looking at this 👍

I left some feedback inline, which I think mostly matches the feedback I gave in the stream, but of course more specific in nature :)

Certainly while this is under development it's fine to keep it as a separate test. As the PR develops you may find it's increasingly similar to what the other test does and can be combined - or not. Regarding specific points:

• I'm aware the muted aspect is distinct from the other test right now, but it's possible that we need to incorporate handling muted streams/topics there too, as a separate improvement.
• As with this PR, it's good to focus on the specific change rather than 'in the future' - unless you're going to include it in a commit straight after! So considering muted users to be an obstacle right now is not important.

As I noted, you should be able to simply use the stream_msg_template fixture. I'd also be interested as to what happened with using the general fixture; the similar test function has logic in the test that translates from types_when_notify_called to a variable, to enable testing the title, but I don't think that should affect the logic - and should probably be in another test.

[neiljp]

user_id, flags, display_recipient and subject are the same through all the test cases you have, at least so far. For that reason it's easier to omit them from the parametrize and focus on the variables that do change - that makes it easier to identify why each test case relates to specific values. The constant values you can leave as defaults based on the fixture, or if they don't work for you for some reason, set them explicitly (and say why).

[Subhasish-Behera]

I included user_id by seeing the model.user_id = user_id of test_notify_users_calling_msg_type but I will be removing user_id totally(as it is not used).
And set the rest of them as you suggested.

[zulipbot]

Hello @zulip/server-notifications members, this pull request was labeled with the "area: notifications" label, so you may want to check it out!

[mounilKshah]

This works just as per expectations on my local machine. I haven't gone through the automated tests yet, but upon manually testing it, everything seemed to work fine 👍

[theViz343]

This looks good! @Subhasish-Behera I've tested it manually and it seems to work great :)

[neiljp]

@Subhasish-Behera Thanks for updating this - I left some feedback now you have extended/combined the tests 👍

[mounilKshah]

I tried this on my local machine and it worked as per expectations. Looks good to me. I have left feedback for the code written to assign value to recipient variable. Apart from Neil's comments, this is something that can be looked into.

[neiljp]

I'm following up on this now.