Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#105 - Added checkbox to registration form for University Supervisor #129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

#105 - Added checkbox to registration form for University Supervisor #129

wants to merge 1 commit into from

Conversation

pehala
Copy link
Contributor

@pehala pehala commented Aug 31, 2016

Overview

  • new Domain class RoleRequest.
    • RoleRequest is created when registrating, but is enabled after confirming account.
  • Added new notifications when creating, approving or declining RoleRequest.
  • RoleRequests can be found under Members tab.
    • Members tab now has 2 tabs inside, Members and RoleRequests.
    • Only Admin can approve, decline RoleRequest.
  • Added Secured annotation to list() method in UserController, since only Admin can access it.

New Files

RoleRequestController.groovy

  • New Controller for handling RoleRequests.
  • list() method is compatible with #121 - Updated topic filter to search in title/description or lead paragraph #126, meaning they have type specified in method instead of form.
  • Only Applicant and Admin can view RoleRequest, while approve/disapprove/list can be done only by Admin.
    • Currently no way for Applicant to view his RoleRequest, expect for direct link, because I couldn't think of good way to incorporate it into User profile.

RoleRequest.groovy

  • New Domain class, used for permission requests.

RoleRequestListenerService.groovy

  • 3 new Listeners for creation/approval/disapproval of RoleRequest.

RoleRequestEvent.groovy

  • Event for transferring parameters to RoleRequest events.

layouts/_listLayout.gsp

  • Layout for Members and RoleRequests based on tabs in User profile.

roleRequest/list.gsp

  • List of RoleRequests with Filter sidebar.

roleRequest/show.gsp

  • Information about specific RoleRequest.

Updated files

BootStrap.groovy

  • Added 2 RoleRequests to dev environment.

UserController.groovy

  • Only Admin can now list the Users.

RegistrationController.groovy

  • Updated register() method to create disabled RoleRequest after saving user.
  • complete() method now shows additional text if user asked for University supervisor permissions.

UserListenerService.groovy

  • UserConfirmed listener now enables both RoleRequest and User and fires RoleRequestCreated event.
  • TimedOut listener now removes both RoleRequest and User.

layouts/_navigation.gsp

  • Members tab is now highlighted even when operating in RoleRequestController.

registration/_form.gsp

  • Added 'I Am Supervisor' checkbox.

user/list.gsp

  • Updated to fit listLayout.

VaclavDedik
VaclavDedik reviewed Sep 9, 2016
View reviewed changes
grails-app/services/com/redhat/theses/listeners/RoleRequestListenerService.groovy

def feed = new Feed(messageCode: 'feed.request.created', args: args, user: e.user).save()

for (User admin : User.list().findAll { user -> Role.ADMIN in user.roles }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use "User.findByRole" or something like that. Grails generates these methods automatically. User.list() retrieves all users from the database imho, which taxes the database.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly, Grails doesn't generate these functions for enums, but I found better way to do it: Generate my own query to filter only Admins.

@pehala pehala force-pushed the #105 branch 2 times, most recently from d82e34b to 22adc4c Compare September 9, 2016 13:48
@jcechace
Copy link
Member

Am I missing something or is this not covered by any unit tests? I think we should have some before merging new functionality.

@VaclavDedik @sbunciak WDYT?

@sbunciak
Copy link

I agree

@pehala pehala force-pushed the #105 branch 2 times, most recently from 0a21b59 to 9434754 Compare November 16, 2016 13:44
@pehala
Copy link
Contributor Author

pehala commented Nov 16, 2016

I added RoleRequestIntegrationSpec, which checks functions in RoleRequestController. I wanted to do also notifications tests but I can't due to that weird MongoDB issue.

@pehala
Added checkbox to registration form for University Supervisor
eb44e7d 
- new Domain class RoleRequest
  - RoleRequest is created when registrating, but is enabled after confirming account.
- Added new notifications when creating, approving or declining RoleRequest
- RoleRequests can be found under Members tab
  - Members tab now has 2 tabs inside, Members and RoleRequests
  - Only Admin can approve, decline RoleRequest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pehala @jcechace @sbunciak @VaclavDedik