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.

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

Refactor the Roles Controller #71

Closed
ShawnClake opened this issue Mar 7, 2017 · 3 comments
Closed

Refactor the Roles Controller #71

ShawnClake opened this issue Mar 7, 2017 · 3 comments
Assignees

Comments

@ShawnClake
Copy link
Owner

ShawnClake commented Mar 7, 2017

This file is quickly becoming a mammoth and needs some form of consistency and standards added in.

How did this happen?

  • I wasn't too familiar with the controller side of October when I started it, so a lot of the code could be considered a 'hack' or there might be better ways to do things.
  • RoleManager, GroupManager, and most of the other big classes received refactors in the beta release. These refactors will make logic in the Roles controller simplified if converted to use the new syntax's.

Potential fixes:

  • ✔ Partials related to groups should only be passed group variables.
  • ✔ Partials related to roles should only be passed role variables.
  • ✔ Partials related to users should only be passed user variables.
  • ✔ Partials related to attaching roles to groups should only be passed role variables. (See point below for finding current group).
  • ✔ Store the currently selected group in the session. Thus, a current group variable should never be passed to a partial.
  • ✔ Store the currently selected role in the session. Thus, a current role variable should never be passed to a partial.
  • ✔ AJAX data attributes API (in partials) should only pass back variables with the names: roleCode, groupCode, userId
  • ✔ Complete the drag and drop functionality. Potentially use this lib: http://interactjs.io/
  • ✔ Unify the render functions. Add helper render functions which call groups of render functions. Remove this nasty syntax that's floating around: https://gyazo.com/61b8ea671c60570b0c9ba7a7aee418aa
  • ✔ Add flash feedback for every action taken.
  • ✔ Improve the naming of functions, in particular, the AJAX functions. Follow the standard onActionTypeOptional() where Action is verbs such as create, update, delete, remove, and Type is the object type such as Role, Group, User. Optional is additional information if needed.
  • ✖ Remove [0] syntax as shown here: https://gyazo.com/75396f5ca9a8813cb1f30b5a52944493 in favor of the reset() function.
  • ✖ (Pushed Back as this is an additional feature, not a refactor) Use Lang
  • ✔ Improve styling and layout
  • ✔ Change the ID of the divs which accept rendered partials to be equivalent to the name of the partial file. For example, if you wanted to render _create_group_form.htm the div's ID would be create_group_form
  • ✔ Change the naming of the partials to follow a standard: _action_type_displaytype_optional.htm
    • _create_group_form.htm
    • _list_roles_table.htm
    • _manage_role_toolbar.htm
    • _manage_group_toolbar.htm
    • _update_group_form.htm
    • _validate_group_form.htm
  • ✔ Add a const list of renderable partials. Then create a render function which takes in an array of these const's and calls the respective render function and returns the made partials.
    • const UE_CREATE_GROUP_FORM = 'create_group_form';
    • const UE_LIST_ROLES_TABLE = 'list_roles_table';
  • ✔ Make it so the toolbars don't scroll. This is annoying and counter intuitive to many users.
  • ✖ (Pushed Back as this is an additional feature, not a refactor) Add a method of adding users to a group other than going to the user backend and checking off their check boxes.
  • ✖ (Pushed Back as this is an additional feature, not a refactor) Add a method of setting a default role for each group for when users are added to a group.

Any other suggestions?

@ShawnClake
Copy link
Owner Author

Completed

  • Add a const list of renderable partials. Then create a render function which takes in an array of these const's and calls the respective render function and returns the made partials.
    * const UE_CREATE_GROUP_FORM = 'create_group_form';
    * const UE_LIST_ROLES_TABLE = 'list_roles_table';

In-Progress

Interactjs Lib has been added

  • Store the currently selected group in the session. Thus, a current group variable should never be passed to a partial.

Added functions for doing this, they aren't utilized yet though

  • Store the currently selected role in the session. Thus, a current role variable should never be passed to a partial.

Added functions for doing this, they aren't utilized yet though

@ShawnClake
Copy link
Owner Author

Completed

  • Change the naming of the partials to follow a standard: _action_type_displaytype_optional.htm
    _create_group_form.htm
    _list_roles_table.htm
    _manage_role_toolbar.htm
    _manage_group_toolbar.htm
    _update_group_form.htm
    _validate_group_form.htm

ShawnClake added a commit that referenced this issue Mar 16, 2017
ShawnClake added a commit that referenced this issue Mar 21, 2017
ShawnClake added a commit that referenced this issue Mar 21, 2017
@ShawnClake ShawnClake mentioned this issue Mar 22, 2017
15 tasks
ShawnClake added a commit that referenced this issue Mar 22, 2017
… Flash and div based error/success messages for controller operations
ShawnClake added a commit that referenced this issue Mar 29, 2017
@ShawnClake
Copy link
Owner Author

Completed and added pushed back items to the push back list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant