Summary

Fix issues with the notification manager and tests.

Review Request #8083 — Created March 29, 2016 and submitted March 30, 2016, 10:47 a.m.

Information

Owner

david

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

9ec2e5b...

Reviewers

Groups

reviewboard

People

Description

The cleanup change to notification manager seems to have been committed in a
weird state. The "moved" files just ended up adding the new versions without
deleting the old, which meant the old tests were still running. Additionally,
the implementation of the notify method was missing a local variable
assignment that was needed.

Testing Done

Ran js-tests.

Issues

Description	From	Last Updated
We can get rid of the const notification above if we use this._notification here. Also, are we safe moving from …	brennie	March 29, 2016, 1:17 p.m.

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/ui/managers/tests/notificationManagerModelTests.js
    reviewboard/static/rb/js/ui/managers/notificationManagerModel.js
    reviewboard/static/rb/js/ui/views/notificationManager.es6.js



Tool: Pyflakes
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/ui/managers/tests/notificationManagerModelTests.js
    reviewboard/static/rb/js/ui/managers/notificationManagerModel.js
    reviewboard/static/rb/js/ui/views/notificationManager.es6.js

reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1)

The issue has been dropped. Show all issues

We can get rid of the const notification above if we use this._notification here.
Also, are we safe moving from _.bind to Function.prototype.bind?

david March 29, 2016, 1:16 p.m.

We can use this._notification here but I'm not sure we can in the onclick handler above it.
We should be safe using Function.prototype.bind. It's supported in all the browsers we care about. The only place where I prefer _.bind (at least for the time being) is when we're defining an anonymous function (since the function () {}.bind() syntax is just funky. A lot of those places probably make sense to have fat arrow functions instead, though.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-3.0.x (0cfa751)