Summary

Support not sending e-mails on WebAPI token creation

Review Request #9258 — Created Oct. 10, 2017 and submitted Nov. 1, 2017, 2:25 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

~~9275~~

Blocks

9259

Commit

de65698...

Reviewers

Groups

reviewboard

People

Description

Currently when a WebAPI token is created, we always send an e-mail
notifying the user of its creation. However, this is not always ideal.
Some extensions and/or integrations require a WebAPI token in order to
communicate with Review Board from some external service (such as
CircleCI). In this case, we do not need to bother the user with a
notification that the token was created.

Djblets now has support for token update and create signals (via
/r/9275). We use these signals to determine if the token generated was
auto-generated. The notifications module has been modified to listen
to the new signals (instead of the generic post_save signal it was
listening to before) and not send e-mails when the token in question
is auto-generated.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
F841 local variable 'webapi_token' is assigned to but never used	reviewbot	Oct. 10, 2017, 4 p.m.
F401 'reviewboard.webapi.signals.webapi_token_created' imported but unused	reviewbot	Oct. 10, 2017, 4 p.m.
At this point it might be nice to turn this into a table with signal, handler, sender and then iterate …	david	Oct. 16, 2017, 4:02 p.m.
We could just have a signal for this in djblets. It's definitely possible that other consumers of djblets.webapi will have …	david	Oct. 17, 2017, noon
F401 'reviewboard.webapi.managers.WebAPITokenManager' imported but unused	reviewbot	Oct. 17, 2017, 12:07 p.m.
F401 'reviewboard.webapi.signals.webapi_token_updated' imported but unused	reviewbot	Oct. 17, 2017, 12:07 p.m.
Maybe a list of tuples?	david	Oct. 20, 2017, 12:46 p.m.
This docstring needs to be updated now.	david	Oct. 20, 2017, 12:46 p.m.
The outbox should default to [] at the beginning of every test. I think you can get rid of this …	david	Oct. 20, 2017, 12:47 p.m.
I think other things assertEqual(len(mail.outbox), 0)	david	Oct. 20, 2017, 12:47 p.m.
This can fit on one line.	chipx86	Oct. 23, 2017, 2:07 p.m.
"WebAPIToken"	chipx86	Oct. 23, 2017, 2:08 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/notifications/tests.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F841 local variable 'webapi_token' is assigned to but never used
```
reviewboard/webapi/models.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F401 'reviewboard.webapi.signals.webapi_token_created' imported but unused
```

Commit:

-	ae136263f6f7459e3ce78b48e25d0026b5b6586c
+	630e90eb4669427a9cc8bfd06de47b092062fe91

Diff:

Revision 2 (+149 -14)

Show changes

	reviewboard/notifications/tests.py
	reviewboard/notifications/email/__init__.py
	reviewboard/notifications/email/signal_handlers.py
	reviewboard/webapi/managers.py
	reviewboard/webapi/models.py
	reviewboard/webapi/signals.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/notifications/email/__init__.py (Diff revision 2)

The issue has been resolved. Show all issues

At this point it might be nice to turn this into a table with signal, handler, sender and then iterate that to do all the connections.

reviewboard/webapi/managers.py (Diff revision 2)

The issue has been resolved. Show all issues

We could just have a signal for this in djblets. It's definitely possible that other consumers of djblets.webapi will have similar wants.

brennie Oct. 16, 2017, 3:56 p.m.

I feel like the wanting email or not is kinda specific to RB itself?

david Oct. 16, 2017, 4:32 p.m.

I mean the created signal, not the handler.

brennie Oct. 16, 2017, 9:16 p.m.

Yeah but we are adding the signal basically only for this reason. We have to have the args the signal will accept ahead of time, and having send_email be one of those is weird. Perhaps it would be better to rename this to autogenerated? I would be comfortable with that in djblets.