Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Fix EZP-24905 Role assignment limitations #397

Closed
wants to merge 1 commit into from
Closed

Fix EZP-24905 Role assignment limitations #397

wants to merge 1 commit into from

Conversation

mhyndle
Copy link
Contributor

@mhyndle mhyndle commented Oct 22, 2015

https://jira.ez.no/browse/EZP-24905

Description

Not working yet. The gist of RoleServerSideView is:

  • The "Assign to..." button calls _pickAssignee() which is used when assigning a role without limitation; this is working.
  • The "Assign to... with subtree limitation" button calls _pickAssigneeLimitSubtree(), used for assigning with limit on subtree. This fires a UDW for selecting subtree, whose contentDiscoveredHandler calls __pickAssigneeLimitSubtreeAssign(). That function should fire a UDW for selecting users/groups to assign to, but this fails silently.
  • The "Assign to... with section limitation" button starts a new view with a section select dropdown. The Assign button in this view should use JS to read the selected section ID (currently hardcoded to 3) and pass it in the button attribute data-role-assignment-section-id. The button calls _pickAssigneeLimitSection() which fires a UDW for selecting users/groups. The UDW opens, but when selecting something it fails with "TypeError: s is undefined oop-min.js:8:1932"

Tasks

  • PHP and templates
  • Rebase once https://jira.ez.no/browse/EZP-24700 is stabilised/merged
  • Fix loading of the second UDW when assigning by subtree limitation (moved to subtasks)
  • Ensure that the "assign by section limitation"-button reads section ID from the dropdown (moved to subtasks)
  • Fix oop-min.js error for section limitation UDW (moved to subtasks)
  • Remove console.log debugging lines (moved to subtasks)

Notes

It's new PR, but it's the same as it was in #383
I have messed that one rebasing it on master, so this PR is the clear one based on master

*
* @return \Symfony\Component\HttpFoundation\Response
*/
public function assignRoleBySectionAction(Request $request, $roleId)
Copy link
Contributor

Choose a reason for hiding this comment

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

assignRoleBySection? I'd go for assingSectionLimitationToRole maybe...
edit: or this is a new feature? can you assing roles by section now?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not new, and I would maybe call it assignRoleBySectionLimitation

Copy link
Member

Choose a reason for hiding this comment

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

assignRoleBySectionLimitationAction then, to stay in sync with other *Action methods.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok. nevermind. depends on how you read it. as i see you could ADD a section limitation to a Role and you could assign A role TO a group of users. With the Limitation suffix looks better anyway.

i read it as "assing this role to a section of users". thats what's looked weird to me.

Copy link
Member

Choose a reason for hiding this comment

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

@crevillo I wouldn't say it like that. It's always about "assigning a role to user/group", whether the assignment is limited or not. The limitation is for the assignment, not for the role.

@mhyndle
Copy link
Contributor Author

mhyndle commented Oct 22, 2015

It should be probably PR made from ezsystems/PlatformUIBundle branch but I have unfortunatelly made it from my fork branch. So I'll ask @glye for help if there will be anything I wouldn't know how to handle it.
For now ping for review @andrerom @lolautruche @bdunogier @yannickroger

@andrerom
Copy link
Contributor

+1

<option value="{{ section.id }}">{{ section.name }}</option>
{% endfor %}
</select>
{# TODO: Section ID 3 is hardcoded below, it should be read from the select above. #}
Copy link
Member

Choose a reason for hiding this comment

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

For reviewers who don't know, this TODO is part of what's moved to a follow-up (to keep the JS heavy stuff separate)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed button's attribute as it won't be used. Section ID will be taken directly from select.

Modified to take roleservice simplification into account.
@glye
Copy link
Member

glye commented Oct 27, 2015

Guess I can't plus-one my own work, but let's say I strongly endorse it.

@andrerom
Copy link
Contributor

ping @yannickroger / @dpobel

@dpobel
Copy link
Contributor

dpobel commented Jan 8, 2016

+1 but this can not be merged until when implement the sub-tasks to add the behavior to the new buttons.
See https://jira.ez.no/browse/EZP-24905#comment-183506

@andrerom
Copy link
Contributor

Seems to have been replaced by #543.

@andrerom andrerom closed this Jul 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
5 participants