I am in the process of upgrading the Administer Users by Role module to Drupal 8. This issue has been created to track progress.

Comments

AdamPS’s picture

Please note that there are many current critical security bugs. I'm a newly appointed co-maintainer and in the process of fixing them. You might be better waiting until that's done before spending longer on D8.

ashrafabed’s picture

Thank you for that information. I did notice a number of issues while porting over the module, but didn't want to fix them all / essentially fork it into a different module, so ported it as-is. Largely was using this project to learn D8, so I'm still interested in updating it when you think it's ready to update.

Here's is the sandbox for the upgrade:
https://www.drupal.org/sandbox/ashrafabed/2380691

And here is the 8.x branch on github: https://github.com/ashabed/administerusersbyrole/tree/8.x-1.x

I believe that it currently provides all of the same functionality as the 7.x-1.x module, with the exception of the .test file - it has not been updated/tested.

I will hold off on further testing until what you're working on is fixed; then I'll rebase / update what needs to be updated.

AdamPS’s picture

@ashrafabed
It's great to have someone working on D8, thanks. I just took a look at your code and - wow, I hadn't realised how different D8 is going to be!

I think you could start work now if you wanted based on the 7.x-2.x-dev branch. I have cleared the backlog of issues, and the code should be mostly stable. There will likely be a few bug fixes still, so you can wait if you prefer.

What I learnt from fixing all the bugs is that you need to have a very thorough understanding of how the core user module works. For example, the function administerusersbyrole_form_user_profile_form_alter comes from reading the code that builds the form and noticing all the fields that have access conditional on admin permissions. The details may well be different in D8, so you need to check carefully.

The function _administerusersbyrole_temp_administer_users is a bit of a hack. In D7, the create user form can run either as admin, or a user registering themselves - and so the code has many branches inside it dependent on checking for admin permission, with no way to override. I hope that a better way is possible in D8 (see #1671182: Ability to better control wether user_register_form is built in admin mode but it isn't fixed). The technique was copied from the subuser module, so you could check if they have D8 code.

There is now an update function in administerusersbyrole.install. I think you can ignore this for D8. It seems reasonable to require users to upgrade from 7.x-1.0-beta to 7.x-2.0 first, and then upgrade to D8. As soon as the new stable release is available, I will mark 7.x-1.0-beta as unsupported.

Please feel free to ask if there is anything you don't understand about the D7 code. A lot of it is quite subtle and took me several attempts to get right!

gvso’s picture

Hi! I have been working to port the version 7.x-2.x to Drupal 8. Here is the code

https://github.com/GVSO/administerusersbyrole

gvso’s picture

Status: Active » Needs review
AdamPS’s picture

@gvso Thanks for the code.

I don't have any experience of D8 and I'm not likely to be using it anytime soon. I don't really even have the knowledge to review the code, but I can at least ask some questions to check everything I learnt from writing D7 had been taken into D8.

I think the next step for someone to volunteer to be a maintainer for D8. If anyone is interested please let me know. We can have more than one even.

As you can see @ashrafabed also supplied some code, although based on the 7.x.1.x branch so would need updating. I think the more people interested and working on it the better - we will get better code. However we need to ensure everyone is working together on a single codebase rather than having separate ones. I can create a D8 branch, although github could be fine for the early stages.

gvso’s picture

Hi @AdamPS, I'd be happy to help you out with this module and yes I used @ashrafabed as a base, but the only difference is that I just implement 7.x-2.x behaviors.

ashrafabed’s picture

@AdamPS, thank you. I would like to volunteer to be a maintainer of the D8 branch - the differences encountered during the D7 to D8 initial code port were surprising. I thought that @gvso had upgraded to D8 from D7 as well, but am happy to hear the work wasn't replicated from scratch.

AdamPS’s picture

Great, I'm happy to make you both maintainers. I'm busy on other things for the next month and will set things up after that. In the meantime, I suggest you work together on making the D8 code in github as good as you can with available time.

  • Automatic tests working.
  • Read the 7.x.2.x code and check each part has a corresponding D8 part (or be clear why one isn't needed).
  • Manual test as many scenarios as you can. What I did was set up a sub-admin and a bunch of other test users and roles. Then set permissions so that the sub-admin should have edit access to only some, and cancel access to some different set. Then try to edit, cancel in different ways and check it all works out.

Many thanks.

gvso’s picture

Awesome! @ashrafabed, is there a way to contact you? So, we can talk about what @AdamPS told us.
Are you in IRC? I didn't see it in your profile. I'm as gvso. I will be out from January 20th until February, so it's will be fantastic if we can do it as soon as possible we can. :)

ashrafabed’s picture

@gvso, I am not currently on IRC, but can be reached via e-mail: (my drupal.org username) @ gmail.com .

I wrote the code for the 7.x-1.x branch, and you updated it to the 7.x-2.x branch . I believe that lends us to the following starting point:
- I review the 7.x-2.x code / ensure that each part has a corresponding D8 part.
- You manual test as many scenarios as possible (I also did this while updating from 7.x-1.x).
- If you would like to work on updating the automated tests, you are more than welcome to work on that piece.

Posting this here so that others who may be interested can see the current status. Please feel welcome to e-mail me.

gvso’s picture

@ashrafabed, ok... that is perfect to me. Actually the automated test was updated, so you can run it and try.

gvso’s picture

Hi! I have tested the module as @AdamPS recommended and it seems working fine for me. However, if someone else could test it again, it would be great

AdamPS’s picture

Thanks for keeping this one moving whilst I was away. My main question now is once we create a D8 branch, how are we going to move forward with general fixing?

If I understand correctly in Drupal core, bugs are checked into D8 then back-ported. However I'm not going to use D8 myself for a good long time yet - so that would end up meaning that I was unable to fix bugs in D7 (which I don't rule out if other maintainers are keen - I mostly took the module on as the previous maintainer no longer had time).

Otherwise, in these early stages do you think it would be feasible to check into D7 as the "master" then port to D8? Or can we let the D8 code live in github for a little while longer? Either way we potentially need to keep track of which patches have yet to be ported.

Suggestions and discussion welcome

I would be willing to have a quick test if anyone could let me have access to a D8 test site with the code running on it.

MegaChriz’s picture

By looking at the code provided in #4 it seems access checks are handled by what looks like altering existing routes. Isn't it much better to use the Entity Access API provided in D8 core and implement hook_entity_access()? The D8 version of User protect (which fulfils a similar use case) also does this.

AdamPS’s picture

@MegaChriz That sounds like a good idea. I'm not currently using D8, so I will it up to the others who are handling it to consider.

In case it's of interest, I'm currently in the process of writing D7 implementations of hook_entity_info_alter and hook_entity_property_info_alter. This will allow entity-based modules such as Views Bulk Operations to operate correctly combined with this module.

AdamPS’s picture

Status: Needs review » Needs work

If anyone wants to move forward on D8 please let me know. The code we have so far seems like a good start. Next steps are:

  • Bring the code up to date with the changes I have made since the original port.
  • Take advantage of new features in D8 core to get better code.
  • Decide on how to maintain the D8 version going forward - whether to make a release at this stage and if so who will maintain it.
ashrafabed’s picture

I did the initial port to D8 and would like to update the port and maintain the module. I'll have it updated within the next week.

pminf’s picture

I've tested the D8 version and it's working as expected besides a little issue: when I enable the module, the user primary tabs with view, edit and – in my case – roles (module "role delegation") links disappear. I checked the module and can't find the reason for this behaviour. Any idea?

I would like to help if someone could give me a hint. Besides this issue I think this module is ready for at least a beta status. Let's get it on so that it "will be released shortly." (quote from modules description page ;-) )

AdamPS’s picture

@pminf Thanks for the offer of help. I can't help you with the hint I'm afraid as I don't yet know anything about D8. Possibly the comment in #15 might solve the problem?

I'm sure it would help to have a D8 dev version at least. This could be a direct copy of the github code.

Before making an alpha release, I would like to raise issues for the known problems. It's possible that the github code has become out of date compared with the D7 version, so it would be useful to check that. We need an issue for #15 and for the problem you are seeing. Provided none of these issue is critical then we can go for alpha.

For a beta release I would like to make sure there has been testing of the key cases - I can draw up a list of everything I have learnt from working on D7 to check the D8 version keeps it working. We should fix all major issues and have function equal to D7. As this module is crucial for security, I don't want to give users the impression it is safe to use until we have some confidence.

The key to move forward is for someone (or more than one person) to agree to be maintainer. People have expressed interest but I guess it's a matter of finding the time. If you want to volunteer I'm happy to give you permissions to check in code. If you check in what's already on github as-is and raise some issues, then that would be a step forwards.

AdamPS’s picture

Assigned: ashrafabed » Unassigned

I have created a D8 dev branch and release

@pminf you now have VCS commit access and so does @ashrafabed

Please raise the existing problems in this thread a separate issue against version D8 dev.

AdamPS’s picture

Version: 7.x-1.0-beta1 » 8.x-2.x-dev
AdamPS’s picture

I created an issue based on #15.
We need one for #19.
Need one for any missing function.

AdamPS’s picture

Status: Needs work » Closed (outdated)
Related issues: +#2807021: Drupal 8 roadmap

Closing this in favour of #2807021: Drupal 8 roadmap - the history here is no longer useful for others to read through.