Continuum

Add user management screens

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 1.1-alpha-1
  • Component/s: Web interface
  • Labels:
    None
  • Complexity:
    Intermediate
  • Number of attachments :
    6

Description

Add a page for listing the users, with options to add/edit/delete user

Users can have an unlimited number of roles (Continuum Permission) like addProject, addUser,... they are already created by the ContinuumInitializer and are static (no new roles/permissions can be added)

Issue Links

Activity

Hide
Teodoro Cue Jr. added a comment -

I've studied this issue and a few questions pop up.

1. How to inject the class Continuum into the action? I've seen a lot of action classes under the package org.apache.maven.continuum.web.action but none of the files contains an annotation for injecting the class Continuum.

2. Looking through xwork.xml, I can see that the class property of action tag doesn't have the same name with that of the class implementing the action. In which, editSchedule instead of EditScheaduleAction. Is this correct? Also, it can't seem to locate users action if I don't put in the fully qualified domain name such as org.apache.maven.continuum.web.action.UsersAction.

3. addUser.jsp seems to be lacking under the webapp folder. Are we just using editUser.jsp for both add and edit operations?

Show
Teodoro Cue Jr. added a comment - I've studied this issue and a few questions pop up. 1. How to inject the class Continuum into the action? I've seen a lot of action classes under the package org.apache.maven.continuum.web.action but none of the files contains an annotation for injecting the class Continuum. 2. Looking through xwork.xml, I can see that the class property of action tag doesn't have the same name with that of the class implementing the action. In which, editSchedule instead of EditScheaduleAction. Is this correct? Also, it can't seem to locate users action if I don't put in the fully qualified domain name such as org.apache.maven.continuum.web.action.UsersAction. 3. addUser.jsp seems to be lacking under the webapp folder. Are we just using editUser.jsp for both add and edit operations?
Hide
Henry S. Isidro added a comment -

Here's some new action classes for the user management jsps as well as changes to the xworks.xml file.

Show
Henry S. Isidro added a comment - Here's some new action classes for the user management jsps as well as changes to the xworks.xml file.
Hide
Emmanuel Venisse added a comment -

1. you must add Continuum class as a requirements of your action in plexus-request.xml. We don't use annotations for the moment in continuum, it's one of our tasks, but you can add annotations in your classes and they will be used when we'll active the descriptor generator

2. The class property used in xwork.xml must be the same of the role-hint of your action declared in plexus-request.xml

3. If your respect what I do for other screens, add and edit screens must be the same jsp with two execution method in the action.

Show
Emmanuel Venisse added a comment - 1. you must add Continuum class as a requirements of your action in plexus-request.xml. We don't use annotations for the moment in continuum, it's one of our tasks, but you can add annotations in your classes and they will be used when we'll active the descriptor generator 2. The class property used in xwork.xml must be the same of the role-hint of your action declared in plexus-request.xml 3. If your respect what I do for other screens, add and edit screens must be the same jsp with two execution method in the action.
Hide
Emmanuel Venisse added a comment -

Henry,

your patch on xwork.xml isn't correct. I don't want to see full classname in class property. It must be a short string that is the same of the role-hint of the action in plexus-request.xml

Show
Emmanuel Venisse added a comment - Henry, your patch on xwork.xml isn't correct. I don't want to see full classname in class property. It must be a short string that is the same of the role-hint of the action in plexus-request.xml
Hide
Henry S. Isidro added a comment -

Yup, I just figured it out with your reply to Teodoro above; will submit a new patch later.

Show
Henry S. Isidro added a comment - Yup, I just figured it out with your reply to Teodoro above; will submit a new patch later.
Hide
Carlos Sanchez added a comment -

The handling of exceptions needs to be improved

+ catch (ContinuumException ce )
+ { + addActionError( "Can't get continuum users: " + ce.getMessage() ); + + ce.printStackTrace(); + + return ERROR; + }

  • printStackTrace() must not be called.
  • Adding the exception message to the action error is not a good idea, it can be any kind of strange message
  • Use the log to print the error
    protected transient static final Log LOG = LogFactory.getLog(MyClass.class);
  • not sure, but the ActionError parameter is probably a key that can be looked up in the localization files to show different errors in different lenguages, and the message for the user is probably "An internal error has occurred", while in the LOG something more meaninful for administrators is used
Show
Carlos Sanchez added a comment - The handling of exceptions needs to be improved + catch (ContinuumException ce ) + { + addActionError( "Can't get continuum users: " + ce.getMessage() ); + + ce.printStackTrace(); + + return ERROR; + }
  • printStackTrace() must not be called.
  • Adding the exception message to the action error is not a good idea, it can be any kind of strange message
  • Use the log to print the error protected transient static final Log LOG = LogFactory.getLog(MyClass.class);
  • not sure, but the ActionError parameter is probably a key that can be looked up in the localization files to show different errors in different lenguages, and the message for the user is probably "An internal error has occurred", while in the LOG something more meaninful for administrators is used
Hide
Emmanuel Venisse added a comment -

Please, don't use LogFactory but plexus logger instead. Plexus logger is our standard for all maven app.

In action class, you can't extends AbstractLogEnabled because actions extends ActionSupport. We can create an abstract action class that implement the plexus logger interface and all actions will use it.

Show
Emmanuel Venisse added a comment - Please, don't use LogFactory but plexus logger instead. Plexus logger is our standard for all maven app. In action class, you can't extends AbstractLogEnabled because actions extends ActionSupport. We can create an abstract action class that implement the plexus logger interface and all actions will use it.
Hide
Jesse McConnell added a comment -

fyi, the abstract class and test cases are a part of a patch I made on CONTINUUM-559, if emmanuel is happy with that we can apply that patch to this branch too maybe, it makes logging simple, just getLogger().info(), etc.

Show
Jesse McConnell added a comment - fyi, the abstract class and test cases are a part of a patch I made on CONTINUUM-559, if emmanuel is happy with that we can apply that patch to this branch too maybe, it makes logging simple, just getLogger().info(), etc.
Hide
Emmanuel Venisse added a comment -

It isn't CONTINUUM-559 but CONTINUUM-759.

I'm happy with that but with some minor comments described in CONTINUUM-759

Show
Emmanuel Venisse added a comment - It isn't CONTINUUM-559 but CONTINUUM-759. I'm happy with that but with some minor comments described in CONTINUUM-759
Hide
Henry S. Isidro added a comment -

Here's a new patch with the imporved logging using Jesse's patch to CONTINUUM-759. It also includes a combined user add and edit screen.

Show
Henry S. Isidro added a comment - Here's a new patch with the imporved logging using Jesse's patch to CONTINUUM-759. It also includes a combined user add and edit screen.
Hide
Jesse McConnell added a comment -

henry, check out CONTINUUM-781

based on some feedback from trygve I moved that base class to the plexus-xwork-integration.

The AbstractContinuumAction can be replaced with PlexusActionSupport from the latest snapshots of that dependency, a patch for that is on 781 as well..

it ought to make life a little easier in the long run

cheers!

Show
Jesse McConnell added a comment - henry, check out CONTINUUM-781 based on some feedback from trygve I moved that base class to the plexus-xwork-integration. The AbstractContinuumAction can be replaced with PlexusActionSupport from the latest snapshots of that dependency, a patch for that is on 781 as well.. it ought to make life a little easier in the long run cheers!
Hide
Henry S. Isidro added a comment -

File Attached: CONTINUUM-771-continuum-webapp-using-PlexusActionSupport.patch

Here's an updated patch with working user management screen. This implements a straightforward permission management for each user (only add and delete with no zones or roles). This patch also uses Jesse's patch in CONTINUUM-781.

Show
Henry S. Isidro added a comment - File Attached: CONTINUUM-771-continuum-webapp-using-PlexusActionSupport.patch Here's an updated patch with working user management screen. This implements a straightforward permission management for each user (only add and delete with no zones or roles). This patch also uses Jesse's patch in CONTINUUM-781.
Hide
Carlos Sanchez added a comment -

Applied the patch in the branch, please look to CONTINUUM-783 for improvements on link construction

Show
Carlos Sanchez added a comment - Applied the patch in the branch, please look to CONTINUUM-783 for improvements on link construction
Hide
Carlos Sanchez added a comment -

Applied again for missing files.
Other comments:
Year of copyright must be {year created}-{year last modified}, so for new files it must be just 2006
Missing javadoc comments in action classes
All other classes have the @version $Id$ tag, so it must be added too for consistency

Show
Carlos Sanchez added a comment - Applied again for missing files. Other comments: Year of copyright must be {year created}-{year last modified}, so for new files it must be just 2006 Missing javadoc comments in action classes All other classes have the @version $Id$ tag, so it must be added too for consistency
Hide
Teodoro Cue Jr. added a comment -

Attached patch fixed for NPE during action loading(because dependencies were not injected) and "add" links inside addUserRole.jsp.

Show
Teodoro Cue Jr. added a comment - Attached patch fixed for NPE during action loading(because dependencies were not injected) and "add" links inside addUserRole.jsp.
Hide
Henry S. Isidro added a comment -

Attached File: CONTINUUM-771-continuum-webapp-menu.patch

This patches the file Menu.jsp and Continuum.properties. It adds the users management link to the menu when an administrator is logged in.

Show
Henry S. Isidro added a comment - Attached File: CONTINUUM-771-continuum-webapp-menu.patch This patches the file Menu.jsp and Continuum.properties. It adds the users management link to the menu when an administrator is logged in.
Hide
Henry S. Isidro added a comment -

Attached File: CONTINUUM-771-continuum-webapp-refactored_user_management_actions.patch

Refactored the user management actions so that they will be reusable.

Show
Henry S. Isidro added a comment - Attached File: CONTINUUM-771-continuum-webapp-refactored_user_management_actions.patch Refactored the user management actions so that they will be reusable.
Hide
Carlos Sanchez added a comment -

Applied patches but the last one that doesn't apply correctly. Also the solution goes through using maven-user as stated in CONTINUUM-800

Show
Carlos Sanchez added a comment - Applied patches but the last one that doesn't apply correctly. Also the solution goes through using maven-user as stated in CONTINUUM-800

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: