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

CommentFileSizeAuthor
time_based_user_registration.png42.62 KBxaiwant

Comments

xaiwant created an issue. See original summary.

xaiwant’s picture

Issue summary: View changes
xaiwant’s picture

Assigned: Unassigned » xaiwant
Status: Needs review » Needs work
xaiwant’s picture

Issue summary: View changes
xaiwant’s picture

Assigned: xaiwant » Unassigned
Status: Needs work » Needs review

made conditional based changes to display user registration form only on mentioned date.

renrhaf’s picture

Title: Time Based Registration » [D7] Time Based Registration
PA robot’s picture

Issue summary: View changes
Status: Needs review » Needs work

There 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.

xaiwant’s picture

Status: Needs work » Needs review

Gone 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 comment
6 | 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
---------------------------------------------------------------------------

grapefruitjuice’s picture

Status: Needs review » Needs work

Hi xaiwant,
I've manually reviewed your module:
- Please use hook_form_FORM_ID_alter instead of global hook_form_alter

function time_based_registration_form_alter(&$form, &$form_state, $form_id) {

- 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.

klausi’s picture

Status: Needs work » Needs review

@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?

grapefruitjuice’s picture

Hi @klausi,
Seems that all OK with other issues and this can be RTBC.

th_tushar’s picture

Automated 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

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements.
3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.
README.txt/README.md
No: Does not follow the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
No: Does not follow the guidelines for project length and complexity.
Secure code
Yes: Meets the security requirements.
Coding style & Drupal API usage

* 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 of hook_permission().

* Use system_settings_form() for admin configuration form. Check this here

* variable_set() can be removed from time_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.

th_tushar’s picture

Status: Needs review » Needs work
Issue tags: +PAreview: single application approval
Code too simple
This project does not demonstrate sufficient hook usage and/or API integration, therefore it is only eligible for a one-time promotion to a full project. You can find more info at Working with the Drupal API.

This 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.

xaiwant’s picture

Assigned: Unassigned » xaiwant

@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

/**
 * Implements  hook_form_FORM_ID_alter().
 */
function time_based_registration_form_user_register_form_alter(&$form, &$form_state, $form_id) {

- 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.

xaiwant’s picture

Status: Needs work » Needs review

@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.

th_tushar’s picture

No need to work on Code long/complex enough for review. :-)

miechiel’s picture

Status: Needs review » Needs work

Hi xaiwant,

I manually reviewed your module:

  1. The function _tbr_configuration_validate should be changed to _time_based_registration_configuration_validate to avoid future name space conflicts.
  2. In hook_help you
    $output .= '<p>' . t('Validate the node title By:') . '</p>';
    

    I don't think this is the correct text.

The first item needs work the second one is not a critical issue.

xaiwant’s picture

Assigned: xaiwant » Unassigned
Status: Needs work » Needs review

@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

  $form['#validate'][] = '_time_based_registration_configuration_validate';
/**
 * Validating the time_based_registration configuration form.
 */
function _time_based_registration_configuration_validate($form, &$form_state) {

In hook_help you

$output .= '' . t('Validate the node title By:') . '';
corrected, removed the unwanted line

updated the codebase.

xaiwant’s picture

Issue summary: View changes
rakesh.gectcr’s picture

Status: Needs review » Reviewed & tested by the community

Looks Fine , I am moving to RTBC, will see what git admin says.

xaiwant’s picture

xaiwant’s picture

Issue summary: View changes
xaiwant’s picture

Issue summary: View changes
th_tushar’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -PAreview: review bonus

Hi @xaiwant,

The module is not recognizing the Date API module even if it exists. It should be

dependencies[] = date_api

instead of

dependencies[] = date:date_api

In time_based_registration.module, due to below code, there is a warning in editor,

$time_based_from = strtotime(variable_get('tbr_config')['from']);
$time_based_to = strtotime(variable_get('tbr_config')['to']);

should be as below

$tbr_config = variable_get('tbr_config');
$time_based_from = strtotime($tbr_config['from']);
$time_based_to = strtotime($tbr_config['to']);
drupal_set_message(t('User registration Allowed between  @tbr_from - @tbr_to', array(
        '@tbr_from' => variable_get('tbr_config')['from'],
        '@tbr_to' => variable_get('tbr_config')['to'],
      )), 'warning');

change to

drupal_set_message(t('User registration Allowed between  @tbr_from - @tbr_to', array(
        '@tbr_from' => $tbr_config['from'],
        '@tbr_to' => $tbr_config['to'],
      )), 'warning');

Also in time_based_registration.admin.inc,

$tbr_config = variable_get('tbr_config');
$from = $tbr_config['from']

instead of

variable_get('tbr_config')['from'];
ariable_get('tbr_config')['to'];

Otherwise looks good to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

xaiwant’s picture

Thanx @th_tushar i will work on #25

xaiwant’s picture

Status: Needs work » Needs review

@th_tushar, As you have mentioned on #25 made changes and committed the code.

please pull up latest code. moving to Need Review.

th_tushar’s picture

Assigned: Unassigned » naveenvalecha
Status: Needs review » Reviewed & tested by the community

One finding,

$tbr_config = variable_get('tbr_config');
$time_based_from = strtotime($tbr_config['from']);
$time_based_to = strtotime($tbr_config['to']);

A $tbr_config should 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.

xaiwant’s picture

@th_tushar as per comment, handling array checks and data existance.

Waiting for Final Review from @naveenvalecha

updated the latest code base.

xaiwant’s picture

@here any update

xaiwant’s picture

Issue summary: View changes
naveenvalecha’s picture

Assigned: naveenvalecha » Unassigned
Status: Reviewed & tested by the community » Needs work

Review of the 7.x-1.x branch (commit 7b0762a):

  • PHP Fatal error: Can't use function return value in write context in ./time_based_registration.module on line 58

    Errors parsing ./time_based_registration.module
  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
  • DrupalPractice has found some issues with your code, but could be false positives.

    <p></p>
    <p>FILE: ...ww/drupal-7-pareview/pareview_temp/time_based_registration.admin.inc<br>
    --------------------------------------------------------------------------<br>
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE<br>
    --------------------------------------------------------------------------<br>
     28 | WARNING | Unused variable $check_user.<br>
    --------------------------------------------------------------------------</p>
    

    Time: 39ms; Memory: 4Mb<br>
    
  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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.

</code></p><code>
<p>FILE: ...ww/drupal-7-pareview/pareview_temp/time_based_registration.admin.inc<br>
--------------------------------------------------------------------------<br>
FOUND 1 ERROR AFFECTING 1 LINE<br>
--------------------------------------------------------------------------<br>
 31 | ERROR | [x] Whitespace found at end of line<br>
--------------------------------------------------------------------------<br>
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY<br>
--------------------------------------------------------------------------</p>
<p>FILE: ...r/www/drupal-7-pareview/pareview_temp/time_based_registration.module<br>
--------------------------------------------------------------------------<br>
FOUND 3 ERRORS AFFECTING 2 LINES<br>
--------------------------------------------------------------------------<br>
 60 | ERROR | [x] Spaces must be used to indent lines; tabs are not<br>
    |       |     allowed<br>
 60 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found 1<br>
 61 | ERROR | [x] Whitespace found at end of line<br>
--------------------------------------------------------------------------<br>
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY<br>
--------------------------------------------------------------------------</p>

Time: 91ms; Memory: 6.25Mb<br>



Source: http://pareview.sh/ - PAReview.sh online service

Manual Review :

  1. This is the better permission name "administer time based registration" than time based registration admin control
  2. time_based_registration_settings_form : use two variables tbr_config_from and tbr_config_to instead of one tbr_config
  3. (*)_time_based_registration_configuration_validate : Remove the prefix _ from the function name as its already prefixed with the module name.Also provide the appropriate fom_set_error message.
  4. Use single quotes over double quotes in the whole code.
  5. Suggested change, change the menu path more user friendly, use admin/config/people/time-based-registration instead of admin/config/people/time_based_registration
  6. time_based_registration_form_user_register_form_alter : Use default values with the tbr_config variable.
xaiwant’s picture

Status: Needs work » Needs review
  1. This is the better permission name "administer time based registration" than time based registration admin control
  2. Added as per Suggestion.

  3. time_based_registration_settings_form : use two variables tbr_config_from and tbr_config_to instead of one tbr_config
  4. These changes has been made as per #th_tushar. follow https://www.drupal.org/node/2709101#comment-11247087

  5. (*)_time_based_registration_configuration_validate : Remove the prefix _ from the function name as its already prefixed with the module name.Also provide the appropriate fom_set_error message.
  6. Remove _Prefix. as module name is already using as per Prefix.

  7. Use single quotes over double quotes in the whole code.
  8. Followed single quotes on required place.

  9. Suggested change, change the menu path more user friendly, use admin/config/people/time-based-registration instead of admin/config/people/time_based_registration
  10. Changed menu path.

  11. time_based_registration_form_user_register_form_alter : Use default values with the tbr_config variable.
  12. These changes has been made as per #th_tushar. follow https://www.drupal.org/node/2709101#comment-11247087

Followed paraview.sh coding standard & instruction.

naveenvalecha’s picture

#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

xaiwant’s picture

@naveenvalecha updated as per my understanding. Updated Codebase.
Needs Review

naveenvalecha’s picture

Status: Needs review » Reviewed & tested by the community

Manual Review( Read commit 5aa3b1d6cf070ce804d0a6d9ff1279d29dc6ef08) :

  1. Use useful & proper commit messages,these are not useful https://www.drupal.org/node/2709089/commits
  2. time_based_registration_form_user_register_form_alter : Use durpal_strtotime instead of strtotime
  3. The path admin/config/people/time_based_registration needs to be updated in Readme.txt
  4. Suggestion in #34 still applied

Nothig major jumped at me.looks good to me.

naveenvalecha’s picture

Status: Reviewed & tested by the community » Fixed

I 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

xaiwant’s picture

Thanx @naveenvalecha,I will follow #36 comment and update accordingly in latest codebase.

THANK YOU FOR GIVING "FULL PROJECT" ACCESS

naveenvalecha’s picture

THANK YOU FOR GIVING "FULL PROJECT" ACCESS

Updated #37, Sorry i forget the single project promote tag.

xaiwant’s picture

No Problem @naveenvalecha. will come up with better idea and better Developing skill.

Thanx

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.