- **status**: review --> in-progress - **Reviewer**: Dave Brondsema - **Comment**:
I found some ways this can be better, but in general this is working just as it should. Performance issue: `my_projects_by_role_name` returns a mongo query cursor, so calling `.count()` on it (twice) and `.first()` will run 3 separate queries. Better to save it to a list first (with `.all()` or `list(...)` and then access `len(...)` and `projects[0]`. Or, even better probably, would be to change `my_projects_by_role_name` to run the `.all()` and return a list for you. Then nobody else using that method will risk the same problem again in the future. It looks like existing uses of `my_projects_by_role_name` probably won't need to be modified to work with that (maybe a test though). Small style note: you don't need `is not None` most of the time. For example, `if note.user_role`will work too and is simpler to read. In `test_get_site_notification_with_page_tool_type_page_regex` it'd be better to patch the `request.url` instead of `re.search`. Then `re.search` will run and you can make sure the pattern matching against urls works as expected. And instead of `request.url` I think it'd be more useful to match against `request.path_qs` which doesn't include the hostname parts. There's no need to match things against the url hostname, and using `path_qs` will let us write regexes that anchor to the beginning of the path, like `^/u/` to show on any pages in the /u/ neighborhood. --- ** [tickets:#8023] More flexibility to the site admin notifications** **Status:** in-progress **Milestone:** unreleased **Labels:** notifications sf-current sf-4 42cc **Created:** Thu Nov 19, 2015 10:50 PM UTC by Dave Brondsema **Last Updated:** Tue Dec 08, 2015 11:33 AM UTC **Owner:** Igor Bondarenko Site notifications would be more useful if they had options to control who they were shown to and on what pages. For the user, some obvious options are show to everyone, or show to project devs/admins. I can't think of others right now. For the pages, tool type(s) could be a useful selection. But all pages are a tool (plus I'd like this to be able to work with external code that can integrate with Allura) so a url regex would be a good option too. That'd cover lots of possibilities (specific neighborhood, specific project, auth pages, create project page, etc). --- Sent from forge-allura.apache.org because [email protected] is subscribed to https://forge-allura.apache.org/p/allura/tickets/ To unsubscribe from further messages, a project admin can change settings at https://forge-allura.apache.org/p/allura/admin/tickets/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.
