model: added placeholder for tables in notifications.

[plugyawn]

What does this PR do? Added a soup_to_formatted_soup function that takes the soup and formats it so the notification doesn't contain any unwanted text.

Currently, it only handles tables (sets it to [MxN table attached]) but in the future it will handle emojis, \me, etc too.

Partially fixes issue #1174.

https://chat.zulip.org/#narrow/stream/206-zulip-terminal/topic/Check.20formatting.20of.20markdown.20in.20notifications.20.23T1174.20.23T1183 Tested?

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

Commit flow

• first commit adds soup_to_formatted_soup() function to boxes.py and calls it from the notify_user function.

Notes & Questions

Interactions

Visual changes Present: Past:

For this message:

[zulipbot]

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

[plugyawn]

Could you discuss why you think this belongs in MessageBox as a classmethod?

My bad, I was working with soup2markup and it missed my mind that it doesn't fit into MessageBox. I'll put it in helpers.py.

a first step might be to refactor the existing spoiler process into such a function first (or a function to be called from it), then extend to take into account the extra behavior.

This I was planning on doing, but was confused whether to add that in a separate PR and focus this PR on only fixing the table versus making one PR to both refactor and then fix the tables.

There are specific variables in the current code that don't appear at first glance to be used yet, which makes this commit more confusing than necessary to read. Those should be added when they are required.

Understood, I'll change those.

The added bonus of a specific function should be that adding a test for that function should be a lot simpler, and could be applied to spoilers too in future.

Got it! I've been looking forward to writing tests for a while now. : ) So this works out nicely.

Each piece of behavior could itself be a function call from there, though I can see you have things set up for a design mirroring the soup2markup function right now?

I have, yes, but right now my plan is to use individual functions to fix each of the issues with tables, emojis, etc, and then wrap all of them in another function, so it's neater and easier to read. I am concerned about some amount of pointless verbosity, though, so I'd like your opinion.

[plugyawn]

Just updated. Please ignore the lines where the out-of-narrow code are adjusted, I'll fix that.

[plugyawn]

@neiljp I changed the commit flow, could you check if it's fine?

[zulipbot]

Heads up @plugyawn, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.