Page MenuHomePhabricator

Don't send coordinator comment notification email if user isn't currently the coordinator
Open, MediumPublic

Description

We had a case this week where an application for which a user was _previously_ the coordinator received an application. Because the platform sends emails to the last coordinator to make a change to the application (https://github.com/WikipediaLibrary/TWLight/blob/master/TWLight/emails/tasks.py#L186), they received an email despite not currently being the coordinator for this partner.

We should do a check before sending this email that the coordinator we're about the email is also the _current_ coordinator.

Event Timeline

Restricted Application added subscribers: Sadads, Aklapper. · View Herald TranscriptJun 24 2019, 8:50 AM
Samwalton9 triaged this task as Medium priority.Jun 26 2019, 2:04 PM
lalit97 claimed this task.Feb 5 2020, 9:11 AM
Samwalton9 edited projects, added Patch-For-Review; removed Google-Code-in-2019.
Samwalton9 moved this task from Doing to Done on the Library-Card-Platform board.Apr 8 2020, 1:21 PM
This comment was removed by Samwalton9.
Samwalton9 moved this task from Done to Doing on the Library-Card-Platform board.Apr 8 2020, 1:22 PM

I have put this task back to In Progress because I merged the PR before time. @lalit97, can you still take care of this task and create a new PR? We can help you out in case you get stuck in a problem.

I have put this task back to In Progress because I merged the PR before time. @lalit97, can you still take care of this task and create a new PR? We can help you out in case you get stuck in a problem.

No problem I have raised a new PR at https://github.com/WikipediaLibrary/TWLight/pull/506. Also mentioned the issue I was facing in the comments :)

I think a rewrite of the class setup might be in order. While that is in scope of the task, it's pretty clear that this isn't the "good-first-task" that we thought it was. We have significant work on the test suite in the backlog anyway, so you might just want to walk away from this one, unless test design in itself is of interest to you.

I put this in the waiting for changes column: this isn't waiting for changes to the pr, it's waiting for other changes to the test suite.

I think a rewrite of the class setup might be in order. While that is in scope of the task, it's pretty clear that this isn't the "good-first-task" that we thought it was. We have significant work on the test suite in the backlog anyway, so you might just want to walk away from this one, unless test design in itself is of interest to you.

Okay I am leaving it here for now. If I understand any tasks related to this properly then I will try to add my contribution :)

We're going to prioritise T263850 soon, so hopefully you can revisit this work before too long :)

Okay I will look into it :)

Moving this into the backlog pending that task being completed.

Samwalton9 removed lalit97 as the assignee of this task.Jan 5 2021, 10:04 AM
Samwalton9 removed a project: good first task.
Samwalton9 removed a project: Patch-For-Review.
Samwalton9 added a subscriber: lalit97.