[Task]: Pastebin - Include link to any attachment in any action

chrstinalin commented 1 month ago

Description

When an attachment is included in an action, the email to the developer should include a link to view the attachment.

Acceptance Criteria

### Milestones/checkpoints
- [x] The email includes a link to view attachments (if any are created).

Checks

[X] If the issue is ready to work on, I have removed the "needs:info" label.
[X] If I have identified that the work is specific to a repository, I have removed "repository:addons-server" or "repository:addons-frontend"

┆Issue is synchronized with this Jira Task

wagnerand commented 1 month ago

This should be "in any action" (that triggers an email), not just reviewer replies, at the very least rejections as well.

ioanarusiczki commented 1 month ago

I did not finish testing this but I should update with some things noticed so far:

downloading attachments from dev hub throws a 404 with a developer account (but it's working with admin). Downloads from rev tools pages are working now (I only checked with an admin account). Mmm, don't know what is going on yet.
I checked emails for approve, reject, delay reject, force disable, force enable actions. Everywhere I see the new line attached, for example "An attachment was provided. To respond or view the file, visit https://addons-dev.allizom.org/developers/addon/blue-overdose-theme3/versions." Maybe to be more specific since approve or reject won't display a count next to review history - it should be "... visit Review History from https://addons-dev.allizom.org/developers/addon/blue-overdose-theme3/versions"
force disable with file attachments won't be displayed in dev hub for the developer. An example.
force enable with file attachments will be available in review history for the developer but it won't be anywhere visible for reviewers. Example

I still need to look over some reviewer actions with abuse reports attached (or appeals) too. And I'm going to file tomorrow separate followups if needed.

chrstinalin commented 1 month ago

downloading attachments from dev hub throws a 404 with a developer account (but it's working with admin). Downloads from rev tools pages are working now (I only checked with an admin account). Mmm, don't know what is going on yet.

Filed #15062

force disable with file attachments won't be displayed in dev hub for the developer. An example. force enable with file attachments will be available in review history for the developer but it won't be anywhere visible for reviewers. Example

Looks like force disable/enable are not surfaced in review history, presumably by design? Would it be better to disable attachments for these actions to avoid this confusion? @diox

diox commented 1 month ago

Force disable/enable apply on the whole add-on, which is why they are not easy to see for developers. I'll leave to @wagnerand whether or not we care not to allow adding attachments to those actions.

wagnerand commented 1 month ago

Ideally, we would allow attachments for force-disabling and -enabling, just to have that opportunity. However, I can't think of a scenario at the moment where we would need that and given that it would create visibility issues for developers, it's better to not allow it for the moment. When we do need in the future, we would make sure it's properly accessible to developers as well.

ioanarusiczki commented 1 month ago

I've some more things from today's stage testing:

I don't know if that's expected: developers can see attachments for clear pending rejections or unrejects

pending rejection cleared unreject

going over abuse reports/appeals, if zip or txt files can be attached to any reviewer action, this is applied for approve/ignore actions from Resolve Reports. These would be attachments to be viewed only by reviewers (they don't go as links to any email, as it would be for a Comment).
for an author's appeal resolved with reject or approve: checking the email sent to developers, the new line is present and files are attached in dev hub. (this is expected)
an author appeal could be resolved in rev tools with Deny action. The email sent has "An attachment was provided. To respond or view the file, visit https://addons.allizom.org/developers/addon/test_for_email_checks/versions." ( applied on version 4.0 in this case) but this is not an action visible for a developer in dev hub.
I can download the attachments from rev tools as an admin, admin reviewer, addon reviewer but I get a 404 with theme and content reviewers.

chrstinalin commented 1 month ago

going over abuse reports/appeals, if zip or txt files can be attached to any reviewer action, this is applied for approve/ignore actions from Resolve Reports. These would be attachments to be viewed only by reviewers (they don't go as links to any email, as it would be for a Comment).

an author appeal could be resolved in rev tools with Deny action. The email sent has "An attachment was provided. To respond or view the file, visit https://addons.allizom.org/developers/addon/test_for_email_checks/versions." ( applied on version 4.0 in this case) but this is not an action visible for a developer in dev hub.

I took a closer look and from what I can tell, log visibility to the developer is based on the activity log's action as a constant only determined after the reviewer action is taken. It may be better to simply prevent the developer from being notified an attachment was included on an action they can't see in DevHub rather than remove the opportunity to attach files entirely (which better aligns with Andreas' https://github.com/mozilla/addons/issues/15018#issuecomment-2386378653 anyway)? @wagnerand @diox

wagnerand commented 3 weeks ago

going over abuse reports/appeals, if zip or txt files can be attached to any reviewer action, this is applied for approve/ignore actions from Resolve Reports. These would be attachments to be viewed only by reviewers (they don't go as links to any email, as it would be for a Comment).

We can disable attaching zip/txt for these actions.

an author appeal could be resolved in rev tools with Deny action. The email sent has "An attachment was provided. To respond or view the file, visit addons.allizom.org/developers/addon/test_for_email_checks/versions." ( applied on version 4.0 in this case) but this is not an action visible for a developer in dev hub.

So, resolving an appeal from an add-on author does not show up in the devhub, but only triggers an email? Is that expected @abyrne-moz? Does that email include reviewer comments? If not, we can disable attachments here as well. If so, let's re-discuss the expected communication.

ioanarusiczki commented 3 weeks ago

About the Deny:

a reviewer can leave a comment

unu

the email shows the comment

We can disable attaching zip/txt for these actions.

I'm going to file this https://github.com/mozilla/addons/issues/15087

mozilla / addons