This module allowes it to distribute the actual user-administration to administrators of a certain domain.
Thereby simplyfying the overview of actual users on a domain.
This can be done by an actual page, created by this module or over views.
The latter is currently recommended for larger sites since it allowes for additional filtering and pagersupport.
Simply install the module go to /admin/structure/domain/ selected added tab, configure rights per domain grant user the additional right and log in as said user.
you can now add users, select,edit or delete them as long as they hold roles specified for that very user or domain.
----------------------
Git-Link:
git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/cupcakemuncher/1837236.git domain_rights_management
Project-Link:
http://drupal.org/sandbox/cupcakemuncher/1837236
Participation:
Patch:
http://drupal.org/node/887910#comment-6933262
Comments
Comment #1
bripatand CreditAttribution: bripatand commentedIt seems there are issues with your repository:
I would suggest you make an automatic review of your module here http://ventral.org/pareview
There are empty functions/hooks in you .install module. Unless you intend to implement them, I'd suggest you remove them.
I think it is better to use lowercase and underscore to name your tables. I could not find a standards as such in Drupal docs but most modules seem to respect this convention (and the core as well).
Comment #2
cupcakemuncher CreditAttribution: cupcakemuncher commentedHi bripatand!
Thank you for the prompt feedback and the heads up on my messed up branch and tag-naming.
That one is fixed.
I did not know about the module not supposed to be put in a subfolder, which I fixed as well.
Nameing-Convetion for the tablenames is adhered.
I want to implement the hook_uninstall as to erase all the rights granted to the users, so that
once the module is activated I do not have users already assigned the role of local administrator.
This does not seem to bother other modules, for I did not see such provisions in their hook_uninstall.
Still thinking about implementing this mechanism, but I do not want to mess around a lot with the code while under revision, same with the naming of the database-tables. I think about refactoring and using defined constants to hold the tablesnames, as opposed to having to refactor the entire code using the IDE's search-and-replace-feature.
The typo in the checkout-command really isn't one.
I used the drupal.org's menus and checked the box for no-maintainer and made a straight copy'n'paste form there, so there is a purpose for this typo, I assum protecting the folks checking out.
But as requested, I will change it in my previous post.
I did a review on http://ventral.org/pareview before checking it in.
No errors I assum it was because of the subfolder-issue.
If I am not mistaken it uses the coder-module which I have installed locally.
Unless s.o. points me in the direction on how I can fix the following issues I will continue to ignore them, since even the domain-module fails the testing of the coder-module, on critical-setting and my one passes.
Setting it on normal or minor produces one of the following errors, depending on the setting
(and editing the code as requested does not sooth the coder-modules tests):
The check of the coder-module seems to not recognise a url after the @see-tag.
only one delimiter between @see and the url and it says that @see is to be followed by either a filename.... or a url.
So the url is in there and the coder-module still does not like it.
The codermodule also complains :
Line 36: Use an indent of 2 spaces, with no tabs
in the .module-file.
Even after I indeted the entire code 2 spaces(excluding <?php) and in case it uses the cache, running update.php.
It also complaints that in the following construct(original layout) the last */ should be indeted with 2 spaces:
/**
* some text
* ....
*/
Which would look bad, and the purpose of a styleguide is to produce easy to read, nice to look at, uniform code.
Here once again the domain-module, behaves the same and does not indet the */ with 2 spaces.
All errors pointed at on the minor-setting of the Coder-module are fixed as well as the spaces for the { brakets after the ) brakets and the spaces between parameters and conditions if (...) {...}.
Comment #3
paravibe CreditAttribution: paravibe commentedPlease add a right git link. There is no 7.x-1.x-alpha branch.
Manual review:
The name of your project folder is "domain_rights__management", should be "domain_rights_management".
domain_rights_management.module: line 13, 18, 196 don’t use a CamelCase. Should be local_administration, administrator_domain_user_settings and so on.
line 107, there is no need to specify MENU_NORMAL_ITEM, because it uses by default.
domain_rights_management.install: Should be better formatted as for me. Line 9, your uninstall function is empty.
includes\ConfigurationMenuFormGeneration.inc, includes\HelperFunctions.inc and here also a camelcase.
Comment #3.0
paravibe CreditAttribution: paravibe commenteddomain_rights__management changed into domain_rights_management as requested below.
Comment #4
cupcakemuncher CreditAttribution: cupcakemuncher commentedPerfect timing on your part;) Issue regarding the __ already addressed in my response above.
Was genereated by drupal.org itself.
Thanks for the hint with the wrong branch, totally missed that one.(took the -alpha out manually)
CamelCase-issue addressed.
I leave line 107 in there, does no harm, and everyone knows what type that link is even if he/she does not know the default-value.
I'll get to implementing hook_uninstall() within the next week, so it remains as well.
Regarding the formatting of the .install-file, if you tell me what bothers you, I can fix it.
cheers
cupcakemuncher
Comment #5
vineet.osscube CreditAttribution: vineet.osscube commentedManual Review :
I suggest you to take a look at this page:
http://git.drupal.org/sandbox/cupcakemuncher/1837236.git
Here you can check source code whether it meets drupal coding standards or not, and advise you what to change in your code. You can repeat review after your commits, and can fix those errors.
Comment #6
cupcakemuncher CreditAttribution: cupcakemuncher commentedHi!
I finally got around to refining the style.
Array-Indendtion should be fixed now.
Constants are written in capital letters now.
t() is used.
Escaping ' is circumvented using " in this rare cases.(went with the suggesting of the onlinechecker).
hook_uninstall is implemented.
Controlestatments do have spaces after them now, as do commas and brakets for functioncalls.
I also indented the entire file content, excluding <?php .
Now the onlinechecker complaints about wrong indention(over 90% of the errors now), so ...
I either do not indent the code within a function with an initial 2 spaces, or
you are wrong, I missunderstood you, or the checker is not up to date.
sincerely
cupcakemuncher
Comment #7
cupcakemuncher CreditAttribution: cupcakemuncher commentedComment #7.0
cupcakemuncher CreditAttribution: cupcakemuncher commentedforgot to change branch.
Comment #8
karljohann CreditAttribution: karljohann commentedAutomated review:
http://ventral.org/pareview/httpgitdrupalorgsandboxcupcakemuncher1837236git
Comment #9
cupcakemuncher CreditAttribution: cupcakemuncher commentedHi!
Ok so indenting the entire file with 2 spaces was a misunderstanding then.
Fixed that one.
Removed all the spelling mistakes I found in README.txt.
Fixed an error in hook_domain_insert().
Surfaced due to removing former includes and using module_load_include() instead.
Had my share of trouble with git, especially a quirky setup concerning smartgit, and openssh.
(So don't even try to understand the commits and branches I made there.)
So fixed my toolchain as well.
Removed ALL the mistakes the onlinechecker found and renamed loaderfunction.
Fixed errors in the documentation.
http://ventral.org/pareview/httpgitdrupalorgsandboxcupcakemuncher1837236git
Comment #10
alexmoreno CreditAttribution: alexmoreno commentedHi cupcakemuncher,
thanks for your contribution and effort contributing this module.
I've ran the Code review (drupal module) over your module and, everything seems fine but in this file: configuration_menu_form_generation.inc
The paraview online tool seems to say everything is ok.
http://ventral.org/pareview/httpgitdrupalorgsandboxcupcakemuncher1837236git
Everything else seems fine and it is well commented.
Thanks again.
Comment #11
cupcakemuncher CreditAttribution: cupcakemuncher commentedHi!
And I have to thank you for the compliment.
Isn't as much documentation as I initially wanted to keep, though.
I changed line 357, 245, 219.
The other outputs are triggered by the shorthand '.=' .
The errors are false positives because it is not possible to seperate '.' and '=' within the '.='-shorthand.
Same with domain_rights_management.module Line 18 and following.
Output:
Line 18: Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
'restrict access' => TRUE,
Sincerely
cupcakemuncher
Comment #12
klausiWe are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)
Comment #13
alexmoreno CreditAttribution: alexmoreno commentedcupcakemuncher, as klausi points, I recommend you to review at least three other projects and include them in the body of your application to proceed with your own project approval.
Comment #14
klausi@urwen: you did no manual review, so why do you set this to "needs work"?
Comment #15
alexmoreno CreditAttribution: alexmoreno commentedsorry if I made a mistake with this klausi, these are my first code reviews, i'm just learning alone :-(. I see that I don't exactly know when to set each status.
Comment #16
thelee CreditAttribution: thelee commentedManual review
This is fairly nitpicky, but especially in
includes/configuration_menu_form_generation.inc
, you have many typos related to the word "cancel" (which you spell "cancle"), both in user-facing URLs and in internal references. There are various other nitpicky grammar-related issues (not capitalizing the start of drupal_set_message() logs, not providing adequate spaces after a full stop) that, while not crucial, add to the overall end-user experience (UX).Other than that, I think the rest of your code (and your module) is pretty solid and robust. The only other suggestion I would have is a more consistent use of db_query versus db_select; from my understanding, db_select is really intended for use for overly complicated queries with many parameters and db_query is much faster, but someone more expert with the database API should comment.
Hope you get cleared/approved soon!
Comment #17
thelee CreditAttribution: thelee commentedComment #18
cupcakemuncher CreditAttribution: cupcakemuncher commentedHi thelee!
Thank you for the compliment.
Unfortunatly the day this project will be cleared might be a long while off.
Currently I do not have time to revisit other projects because my job requires me to learn new systems I am developing for.
Regardless of this fact, I will maintain this module.
Updated all your named issues regarding the end-user-experience.
Using db_select is no accident.
I chose db_select over db_query to provide a better understanding of the code from a programmers perspective,
to guarantee easier changes for future adaptions, to seperate single conditions,etc. and for not having to think about the underlying database (after all there are some subtle differences between the SQL-dialects) .
Only exception to this rule is in hook_uninstall, since that is a simple delete-query. All other queries are....bigger.
Besides AFAIK the major bottleneck of a webapplication is database-access, so I am not really worried about the few extra ticks spent on building the query.
In addition to that I got rid of some unused variables I stumbled upon.(legacy from earlier phases of development)
Patched a major bug regarding form-submission.
(A parameter had to be accepted as by-reference.(see function domain_rights_management_alter_user_form_submit() ) ).
Sincerely
cupcakemuncher
P.s.:Currently ventral.org seems to have issues with the validation. The changes made weren't major though. So I think it would validate just fine. The Coders-module tells everything is ok, except the usual false-positives.
Comment #19
thelee CreditAttribution: thelee commentedcool, i hope no one minds me moving this in to RTBC then.
Comment #20
patrickd CreditAttribution: patrickd commentedComment #21
patrickd CreditAttribution: patrickd commentedYour project page could be prettier, please have a look at the tips for a great project page, you may also use HTML-tags for better structure and screenshots.
Also the Readme looks a lot more informative, maybe you want to keep readme and project page in-sync? (few people are reading readme's)
Please set the 7.x-1.x as the default project branch! To do this go to http://drupal.org/node/1837236/edit/default-branch.
Code looks in general good enough for me but it could use some more inline comments ;-)
Thanks for your contribution!
I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.
Here are some recommended readings to help with excellent maintainership:
You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!
Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
Thanks to the dedicated reviewer(s) as well.
Comment #22
cupcakemuncher CreditAttribution: cupcakemuncher commentedThank you for promoting my project and adding the ability to make full projects.
I will sync readme-file and projectpage asap and thank you for the links.
Comment #23.0
(not verified) CreditAttribution: commentedDocumentation of my involment/participation in the drupal-community.