Module Description:
This module work in simple principle of time. Allows admin to configure guest user registration only between specified time. Is helpful,when admin don't want anonymous user to register in his / her own site & open up site traffic. allow user registration for a duration of time &
How am i restricting..?
- User Registration visiblity made hidden based on current time()
Feature implementation
- adding more option to configure user registration f.e: only on specific day or specific date etc.
Git clone url :
git clone --branch 7.x-1.x https://git.drupal.org/sandbox/xaiwant/2709089.git time_based_registration
Project url :
https://www.drupal.org/sandbox/xaiwant/2709089
Manual Reviews :
https://www.drupal.org/node/2757621#comment-11357261
https://www.drupal.org/node/2680289#comment-10969659
https://www.drupal.org/node/2706963#comment-11104519
https://www.drupal.org/node/2687695#comment-11104869
https://www.drupal.org/node/2702465#comment-11104567
https://www.drupal.org/node/2640372#comment-11108293
| Comment | File | Size | Author |
|---|---|---|---|
| time_based_user_registration.png | 42.62 KB | xaiwant |
Comments
Comment #2
xaiwant commentedComment #3
xaiwant commentedComment #4
xaiwant commentedComment #5
xaiwant commentedmade conditional based changes to display user registration form only on mentioned date.
Comment #6
renrhafComment #7
PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxxaiwant2709089git
Fixed the git clone URL in the issue summary for non-maintainer users.
We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)
Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #8
xaiwant commentedGone through all the errors listing in paraview.sh and fixed it
Below are the list of issues Fixed
FILE: ...www/drupal-7-pareview/pareview_temp/time_based_registration.admin.inc
---------------------------------------------------------------------------
FOUND 33 ERRORS AND 1 WARNING AFFECTING 28 LINES
---------------------------------------------------------------------------
12 | ERROR | [x] Opening brace should be on the same line as the| | declaration
13 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
14 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
15 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
16 | ERROR | [x] Array indentation error, expected 10 spaces but found
| | 12
18 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
21 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
22 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
23 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
24 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
27 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
28 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
29 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
30 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
33 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
34 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
44 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
45 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
47 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
48 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 7
53 | ERROR | [x] Doc comment short description must end with a full
| | stop
56 | ERROR | [x] Opening brace should be on the same line as the
| | declaration
57 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
58 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
59 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
60 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
61 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
61 | WARNING | [ ] Translatable strings must not begin or end with white
| | spaces, use placeholders with t() for variables
61 | ERROR | [ ] Concatenating translatable strings is not allowed, use
| | placeholders instead and only one string literal
61 | ERROR | [x] Concat operator must be surrounded by a single space
61 | ERROR | [x] Concat operator must be surrounded by a single space
61 | ERROR | [x] Concat operator must be surrounded by a single space
61 | ERROR | [x] There should be no white space before a closing ")"
62 | ERROR | [x] Expected 1 newline at end of file; 0 found
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 32 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------
FILE: ...r/www/drupal-7-pareview/pareview_temp/time_based_registration.install
---------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------
14 | ERROR | [x] Expected 1 newline at end of file; 0 found---------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------
FILE: /var/www/drupal-7-pareview/pareview_temp/time_based_registration.info
---------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------
6 | ERROR | [x] Expected 1 newline at end of file; 0 found---------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------
FILE: ...ar/www/drupal-7-pareview/pareview_temp/time_based_registration.module
---------------------------------------------------------------------------
FOUND 41 ERRORS AND 1 WARNING AFFECTING 33 LINES
---------------------------------------------------------------------------
1 | ERROR | [x] Missing file doc comment6 | ERROR | [x] Missing function doc comment, only found file comment
7 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
8 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
9 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
11 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 12
12 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 12
13 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 12
14 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 12
15 | ERROR | [x] Case breaking statement indented incorrectly; expected
| | 10 spaces, found 12
24 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
25 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
26 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
27 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
28 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
29 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
30 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
31 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
40 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
41 | ERROR | [x] Array indentation error, expected 6 spaces but found 8
42 | ERROR | [x] Array indentation error, expected 10 spaces but found
| | 12
43 | ERROR | [x] Array indentation error, expected 10 spaces but found
| | 12
52 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
53 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
54 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
55 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
57 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
58 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 8
58 | ERROR | [x] Expected 1 space after closing parenthesis; found 0
59 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 12
60 | ERROR | [x] Line indented incorrectly; expected 8 spaces, found 12
60 | ERROR | [x] Closing brace indented incorrectly; expected 8 spaces,
| | found 12
60 | ERROR | [x] Expected newline after closing brace
60 | ERROR | [x] Expected 1 space after ELSE keyword; 2 found
61 | ERROR | [x] Line indented incorrectly; expected 10 spaces, found
| | 12
62 | ERROR | [x] Line indented incorrectly; expected 10 spaces, found
| | 12
62 | WARNING | [ ] Translatable strings must not begin or end with white
| | spaces, use placeholders with t() for variables
62 | ERROR | [ ] Concatenating translatable strings is not allowed, use
| | placeholders instead and only one string literal
62 | ERROR | [x] Concat operator must be surrounded by a single space
62 | ERROR | [x] Concat operator must be surrounded by a single space
62 | ERROR | [x] Concat operator must be surrounded by a single space
65 | ERROR | [x] Expected 1 newline at end of file; 0 found
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 40 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------
Comment #9
grapefruitjuice commentedHi xaiwant,
I've manually reviewed your module:
- Please use hook_form_FORM_ID_alter instead of global hook_form_alter
- I guess will be better in You create additional variable with content of variable_get('time_based_registration_config') and then use more shorter record, f.e. $time_config['from'] instead of variable_get('time_based_registration_config')['from']
- I guess on register page/tab, if registration time doesn't setted yet, message "User registration Allowed between -" should be changed on "User registration not allowed at this time" or something else
- Maybe need to add more options for select with time periods, for increasing usability?
Regards.
Comment #10
klausi@GrapefruitJuice: that sound like nice improvement suggestions, but are not application blockers. Any other blocking issue like security vulnerabilities or licensing problems that you found? Or should this be RTBC instead?
Comment #11
grapefruitjuice commentedHi @klausi,
Seems that all OK with other issues and this can be RTBC.
Comment #12
th_tushar commentedAutomated Review
[Best practice issues identified by pareview.sh / drupalcs / coder. Please don't copy/paste all of the results unless they are short. If there are a lot, then post a link to the automated review and mention that problems should be addressed.]
Note that perfect adherence to Drupal Coding Standard is NOT a reason to block an application, except for total disregard of them. However, modules should follow them as closely as possible.
Manual Review
* Access argument for admin/config/people/time_based_registration menu item does not match the custom permission implemented.
'access arguments' => array('time based registration admin control'),should be'access arguments' => array('node title validation admin control'),as per implementation ofhook_permission().* Use
system_settings_form()for admin configuration form. Check this here*
variable_set()can be removed fromtime_based_registration_admin_form_submit()function.The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.
If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.
This review uses the Project Application Review Template.
Comment #13
th_tushar commentedThis is an important criterion so that code integrates well and can be improved over time. I encourage you to continue developing and gaining from the feedback available in the git approval process.
Thank you for you contributions and understanding.
Comment #14
xaiwant commented@GrapefruitJuice, th_tushar thanx for giving your time
Below are the changes i have committed in latest code base.
- Please use hook_form_FORM_ID_alter instead of global hook_form_alter
- I guess will be better in You create additional variable with content of variable_get('time_based_registration_config') and then use more shorter record, f.e. $time_config['from'] instead of variable_get('time_based_registration_config')['from']
Using 'tbr_config'/ 'tbr_config')['from']/ 'tbr_config')['to']
- I guess on register page/tab, if registration time doesn't setted yet, message "User registration Allowed between -" should be changed on "User registration not allowed at this time" or something else
handling the validation if configuration form is not SET
if (!empty($time_based_from) && !empty($time_based_to)) {- Maybe need to add more options for select with time periods, for increasing usability?
As per #13 i'm looking into few more implementation.
Comment #15
xaiwant commented@th_tushar #19
Access argument for admin/config/people/time_based_registration menu item does not match the custom permission implemented.
hook_permission() implemented correctly.
* Use system_settings_form() for admin configuration form.
using system setting form to save the admin configuration.
* variable_set() can be removed from time_based_registration_admin_form_submit() function.
currently we are not using custom submit form so, i removed
README.txt/README.md
Added README.txt in the 7.x branch as per Drupal 7 guidance.
Code long/complex enough for review
Currently working on it.
Comment #16
th_tushar commentedNo need to work on Code long/complex enough for review. :-)
Comment #17
miechiel commentedHi xaiwant,
I manually reviewed your module:
_tbr_configuration_validateshould be changed to_time_based_registration_configuration_validateto avoid future name space conflicts.I don't think this is the correct text.
The first item needs work the second one is not a critical issue.
Comment #18
xaiwant commented@miechiel
The function _tbr_configuration_validate should be changed to _time_based_registration_configuration_validate to avoid future name space conflicts.
function name changed as per above
In hook_help you
$output .= '' . t('Validate the node title By:') . '';corrected, removed the unwanted line
updated the codebase.
Comment #19
xaiwant commentedComment #20
rakesh.gectcrLooks Fine , I am moving to RTBC, will see what git admin says.
Comment #21
xaiwant commentedComment #22
xaiwant commentedAdding PAReview: review bonus
Manual Reviews :
https://www.drupal.org/node/2680289#comment-10969659
https://www.drupal.org/node/2706963#comment-11104519
https://www.drupal.org/node/2687695#comment-11104869
https://www.drupal.org/node/2702465#comment-11104567
https://www.drupal.org/node/2640372#comment-11108293
Ref: https://www.drupal.org/project/issues/search/projectapplications?text=&a...
Comment #23
xaiwant commentedComment #24
xaiwant commentedComment #25
th_tushar commentedHi @xaiwant,
The module is not recognizing the Date API module even if it exists. It should be
dependencies[] = date_apiinstead of
dependencies[] = date:date_apiIn time_based_registration.module, due to below code, there is a warning in editor,
should be as below
change to
Also in time_based_registration.admin.inc,
instead of
Otherwise looks good to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Comment #26
xaiwant commentedThanx @th_tushar i will work on #25
Comment #27
xaiwant commented@th_tushar, As you have mentioned on #25 made changes and committed the code.
please pull up latest code. moving to Need Review.
Comment #28
th_tushar commentedOne finding,
A
$tbr_configshould be checked if it's an array and not empty. Otherwise looks fine to me.Assigning to naveenvalecha as he might have time to take final look at this.
Comment #29
xaiwant commented@th_tushar as per comment, handling array checks and data existance.
Waiting for Final Review from @naveenvalecha
updated the latest code base.
Comment #30
xaiwant commented@here any update
Comment #31
xaiwant commentedComment #32
naveenvalechaReview of the 7.x-1.x branch (commit 7b0762a):
Errors parsing ./time_based_registration.module
This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.
Source: http://pareview.sh/ - PAReview.sh online service
Manual Review :
Comment #33
xaiwant commentedAdded as per Suggestion.
These changes has been made as per #th_tushar. follow https://www.drupal.org/node/2709101#comment-11247087
Remove _Prefix. as module name is already using as per Prefix.
Followed single quotes on required place.
Changed menu path.
These changes has been made as per #th_tushar. follow https://www.drupal.org/node/2709101#comment-11247087
Followed paraview.sh coding standard & instruction.
Comment #34
naveenvalecha#33.2,
Copied from Tushar comment :
A $tbr_config should be checked if it's an array and not empty. Otherwise looks fine to me.he only suggested to check whether the variable is array. I suggested to use two variables instead of using single one. that way you'll not have do the computing to fetch the from and to variable from array
Comment #35
xaiwant commented@naveenvalecha updated as per my understanding. Updated Codebase.
Needs Review
Comment #36
naveenvalechaManual Review( Read commit 5aa3b1d6cf070ce804d0a6d9ff1279d29dc6ef08) :
admin/config/people/time_based_registrationneeds to be updated in Readme.txtNothig major jumped at me.looks good to me.
Comment #37
naveenvalechaI have promoted the project for you. Now you can create the release of the project.
Thanks for your contribution, xaiwant!
I updated your account so you can 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 stay 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.
Edit : I have not granted you for promoting from sandbox to full projects as there was Single project promote tag associated as per #13
Comment #38
xaiwant commentedThanx @naveenvalecha,I will follow #36 comment and update accordingly in latest codebase.
THANK YOU FOR GIVING "FULL PROJECT" ACCESS
Comment #39
naveenvalechaUpdated #37, Sorry i forget the single project promote tag.
Comment #40
xaiwant commentedNo Problem @naveenvalecha. will come up with better idea and better Developing skill.
Thanx