Git Merge Request #186: Removed code for old permissions page (rejected)

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

huzaifafaruqui wants to merge 0 commits from /u/huzaifafaruqui/allura/ to master, 2017-03-16

Now that the /groups URL handles user permissions there was no need for /permissions URL. However, the template project_permissions.html has not been deleted

Commit Date  

Discussion

  • Dave Brondsema

    Dave Brondsema - 2017-03-13

    I think this section isn't being used after all:

    @expose('jinja:allura.ext.admin:templates/project_permissions.html')
def groups(self, **kw):
    return dict()

    Because at the beginning of the class it has self.groups = GroupsController() and that is what is being used. So you can delete those 3 lines and also the project_permissions.html file then.

    Regarding the test_subproject_permissions test that is removed: we don't have any existing test covering a subproject and the "/groups" URL. Do you want to try adding the test_subproject_permissions test back in but changing it to use /groups instead of /permissions URL? The self.app.get line will have to change, and the lines after it would have to change quite a lot. Or you could remove those lines, and the test would only test accessing the page and not making changes. (I think that would be ok since the page should work the same for regular projects and subprojects, and we already have tests for it on a regular project)

     

  • Dave Brondsema

    Dave Brondsema - 2017-03-15

    FYI this commit got merged to master, a bit unintentionally, as part of my mongo 3.4 fixes. Its okay though since the commit itself is good, but the followup changes I mentioned in the previous comment I think we still should do.

     

  • Dave Brondsema

    Dave Brondsema - 2017-03-16
    • Status: open --> rejected
     

Log in to post a comment.