Add ability to see and recycle sent messages by mathjazz · Pull Request #3353 · mozilla/pontoon

mathjazz · 2024-09-18T20:27:54Z

Fix #3246: After the message is sent, it gets stored to the DB and displayed in the Sent panel.
Fix #3247: Add ability to edit sent messages as new

Since @eemeli is on a PTO, I'll be integrating my work on #3247 directly into this PR. That will allow opening sent messages in the Compose panel, revealing more details about them, like the selected filters. It'll also make Sent and Compose panels accessible via URL, which will allow redirecting to the (updated) Sent panel after the message is sent (right now you need to refresh the page to see the sent message, which is annoying).

@bcolsson Could you please take this for a spin on stage? Note that temporarily messages are always sent to the sender, regardless if you click Send to myself or Send. You can also manage messages via Django Admin.

Also included are some under the hood changes that will make code more maintainable:

Updated on Sep 19, 14:20 UTC: I've now integrated the fix for #3247. Related to that:

Compose and Sent panels can now be linked to.
After the message is sent, show it in the Sent panel.

mathjazz · 2024-09-19T17:19:28Z

Known bugs:

Checking Email as message type will result in an error (sending emails on stage is disabled by design)
Messages are stored and sent as (sanitized) HTML for security reasons. When you edit sent messages, HTML is converted back to Markdown, which might not be exactly the same as the Markdown the message was originally written in.

bcolsson · 2024-09-19T23:21:46Z

@mathjazz - testing from a user perspective everything seems nice and functional. Liking it!

flodolo · 2024-09-20T05:54:30Z

When previewing the message, it would be nice to visually differentiate the text from the rest the the UI, e.g.

Questions:

Do we want all locales and all projects selected by default?

Issues:

There are no checks on the fact that Maximum should be bigger than Minimum, or From earlier than To.

mathjazz · 2024-09-20T12:17:00Z

When previewing the message, it would be nice to visually differentiate the text from the rest the the UI, e.g.

Makese sense, I'll play with it a little.

Do we want all locales and all projects selected by default?

That's what the spec says. It's also what we've been using in the recent emails. But not in manual notifications. Maybe leave it for now and adapt if data shows it's not the right default?

There are no checks on the fact that Maximum should be bigger than Minimum, or From earlier than To.

This is checked implicitly, because the number of recipients in cases like this will be zero. Same as above, I'd add an explicit check if this actually turns out to be a problem.

mathjazz · 2024-09-23T12:19:46Z

When previewing the message, it would be nice to visually differentiate the text from the rest the the UI, e.g.

Makese sense, I'll play with it a little.

Made some minor adjustments to the overall styling, including this.

…lace custom code

eemeli

Sorry for the time it took me to get to this. A few small things inline, but I'd appreciate a chance to see this on stage again and take it for a spin?

eemeli · 2024-10-03T14:38:53Z

+      // Keep default middle-, control- and command-click behaviour (open in new tab)
+      if (e.which === 2 || e.metaKey || e.ctrlKey) {
+        return;
+      }


How about shift+click?

We'd need to file a bug for that and change it across all pages with tabs.

The translate frontend already checks for shiftKey when it checks for ctrlKey, so we shouldn't deviate from that more in the places where we do special stuff on clicks.

Suggested change

// Keep default middle-, control- and command-click behaviour (open in new tab)

if (e.which === 2 || e.metaKey || e.ctrlKey) {

return;

}

// Keep default middle-, shift-, control- and command-click behaviour (open in new tab)

if (e.which === 2 || e.metaKey || e.ctrlKey || e.shiftKey) {

return;

}

I meant this: https://github.com/mozilla/pontoon/blob/main/pontoon/base/static/js/tabs.js#L22

I'll update both files.

mathjazz · 2024-10-03T16:18:09Z

Sorry for the time it took me to get to this. A few small things inline, but I'd appreciate a chance to see this on stage again and take it for a spin?

Deployed to stage.

eemeli · 2024-10-03T17:06:36Z

+            </div>
+            <div class="footer">
+                <a href="{{ url('pontoon.messaging.edit_as_new', message.pk) }}" class="edit-as-new"><i class="fa fa-pencil-alt"></i>Edit as new</a>
+                <time>{{ message.sent_at|format_for_inbox }}</time>


I initially didn't see this "sent at" info at all. I would expect for this to be near the top of the message, where we also have the sender, the recipient count & message type.

It's a common pattern to show timestamp under the message bubble in the messaging apps. I think you'll get used to it quickly.

It'll be busy in the header.

codecov-commenter · 2024-10-03T18:07:49Z

Codecov Report

Attention: Patch coverage is 63.63636% with 40 lines in your changes missing coverage. Please review.

Project coverage is 80.44%. Comparing base (b8a2030) to head (2abf95c).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pontoon/messaging/views.py	23.52%	26 Missing ⚠️
pontoon/base/templatetags/helpers.py	30.76%	9 Missing ⚠️
pontoon/messaging/forms.py	71.42%	4 Missing ⚠️
pontoon/messaging/models.py	97.05%	1 Missing ⚠️

Additional details and impacted files

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Also included: * After the message is sent, show it in the Sent panel. * Remove now obsolete updating of custom checboxes (see f6ce601) * Fix comments in the Localizations URL patterns

… Send to myself, don't redirect to Sent.

Co-authored-by: Eemeli Aro <eemeli@gmail.com>

mathjazz · 2024-10-03T18:14:30Z

Updated and re-deployed to stage. I also had to rebase.

eemeli

Looks good to go. Instead of rebasing, doing a merge from main to the PR branch would make the changes easier to review.

mathjazz force-pushed the 3246-messaging-sent branch from b579f21 to 3a3d3e6 Compare September 19, 2024 14:13

mathjazz changed the title ~~Add ability to see sent messages~~ Add ability to see and recycle sent messages Sep 19, 2024

bcolsson approved these changes Sep 19, 2024

View reviewed changes

mathjazz mentioned this pull request Sep 22, 2024

Good riddance #3367

Merged

mathjazz force-pushed the 3246-messaging-sent branch from 4450270 to fe89f15 Compare September 23, 2024 12:12

mathjazz requested a review from eemeli September 23, 2024 12:13

mathjazz force-pushed the 3246-messaging-sent branch from fe89f15 to a8242d0 Compare September 23, 2024 12:21

mathjazz mentioned this pull request Sep 26, 2024

Replace multiple team selector with multiple item selector #3368

Open

mathjazz force-pushed the 3246-messaging-sent branch from a8242d0 to bcbe6e4 Compare September 26, 2024 11:38

mathjazz added a commit to mathjazz/pontoon that referenced this pull request Sep 26, 2024

Merge multiple item selector code from mozilla#3353 and use it to rep…

12d6722

…lace custom code

mathjazz force-pushed the 3246-messaging-sent branch from bcbe6e4 to b9f5b0a Compare September 26, 2024 11:46

eemeli requested changes Oct 3, 2024

View reviewed changes

mathjazz requested a review from eemeli October 3, 2024 16:24

eemeli requested changes Oct 3, 2024

View reviewed changes

mathjazz added 9 commits October 3, 2024 20:08

Add Message model and admin

fc77861

Create MessageForm from the Message model

a0139a7

If form validation fails, error on frontend

1453573

Use <select> element instead of input in multiple item selector widget

9fdaf0b

Save sent messages to the DB

a1a342b

Use form tags in the template

c2d98fb

Show sent messages

b3c4c5f

Load Compose and Sent panels via AJAX, which allows us to link to panels

c457049

Also included: * After the message is sent, show it in the Sent panel. * Remove now obsolete updating of custom checboxes (see f6ce601) * Fix comments in the Localizations URL patterns

Add ability to edit sent messages as new

59273cb

mathjazz and others added 8 commits October 3, 2024 20:09

Temporarily only send messages to the sender. If explicity clicked on…

a19719f

… Send to myself, don't redirect to Sent.

More coherent styling

2c9b82b

Fix projects validation

93f7a3d

Fix typo

df0a73e

Co-authored-by: Eemeli Aro <eemeli@gmail.com>

Unify conditions

981790a

Keep default Shift-behavior when shift-clicking tab and sidebar links

61f750b

Edit as new -> Use as template

a5619dd

Send *test* to myself

2abf95c

mathjazz force-pushed the 3246-messaging-sent branch from 58bb348 to 2abf95c Compare October 3, 2024 18:09

mathjazz requested a review from eemeli October 3, 2024 18:28

eemeli approved these changes Oct 3, 2024

View reviewed changes

mathjazz merged commit 138be1b into mozilla:main Oct 3, 2024

mathjazz deleted the 3246-messaging-sent branch October 3, 2024 20:00

Conversation

mathjazz commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mathjazz commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bcolsson commented Sep 19, 2024

Uh oh!

flodolo commented Sep 20, 2024

Uh oh!

mathjazz commented Sep 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mathjazz commented Sep 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eemeli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eemeli Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

mathjazz Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

eemeli Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathjazz Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

mathjazz Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

mathjazz commented Oct 3, 2024

Uh oh!

Uh oh!

Uh oh!

eemeli Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

mathjazz Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mathjazz commented Oct 3, 2024

Uh oh!

eemeli left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mathjazz commented Sep 18, 2024 •

edited

Loading

mathjazz commented Sep 19, 2024 •

edited

Loading

mathjazz commented Sep 20, 2024 •

edited

Loading

mathjazz commented Sep 23, 2024 •

edited

Loading

eemeli Oct 3, 2024 •

edited

Loading

mathjazz Oct 3, 2024 •

edited

Loading

codecov-commenter commented Oct 3, 2024 •

edited

Loading