Closed mattlindesay closed 6 years ago
Oops. Closed by mistake.
OK no idea why this is not compiling on django master. I don't think it's the code I added. If someone can explain why it's failing I'm happy to spend some time on this.
@mattlindesay recently I have begun removing "master" from our tox.ini and .travis.yml files, replacing them with "1.10". The reason is two-fold. Django dev master no longer allows "from django.core.urlresolvers import reverse", requiring "from django.urls import ..." instead. Secondly, Pinax is tested against "master" mainly to stay on top of the latest changes, but doesn't officially support latest dev. So my suggestion is to remove "master" from the test matrix configurations in both tox.ini and .travis.yml and try again.
See pinax-blog tox.ini for an example of this "master" removal and "1.10" insertion.
Hi again @mattlindesay. Looks like removing "master" and adding "1.10" fixed the build, and adding a test fixed coveralls. Nice work.
Just three small issues remain for this to get merged in:
Since you're looking for a count of unread Threads rather than a list of actual Threads, I think the Django doc optimization guidance is good; use count().
If I understand correctly, this filter produces a count of Threads which have one or more messages not read by user. Proving this works as intended is good to a) show proper use, and b) show other developers what to expect. You've added one test but I think this needs a bit more clarity. Edge cases are good, so maybe three Threads, one unread entirely, one with two messages - one read and another not read, and a third Thread with all messages read by user. Ensure the templatetag produces expected result.
Somehow all file modes have changed, 100644 → 100755. All other Pinax apps use 644, and I think that's standard convention.
Thanks for contributing Matt!
BTW @mattlindesay you (and anyone else) are welcome to join Pinax discussions in the various channels of https://pinax.slack.com.
Thanks for your contribution @mattlindesay. I've incorporated this improvement into pinax-messages via https://github.com/pinax/pinax-messages/pull/42, so closing this pull request.
Additional template tag to show if there are new messages for the user so that you can highlight this on a navbar.