[D7] User Access Timeslot

@Nixou,

Please change git clone URL of the project. The URL you have added in this post is for maintainers. If we use this URL to clone the project then it asks us to provide Nixou@git.drupal.org's password.

Please change the URL so that community members can clone and review the module.

Thanks

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.

Hi @Nixou,

Interesting module. Below are my suggestions based on review.

1) After configuration, when I tried to login it shows below error message.

Sorry, you can't access your account for now, try later.

Currently this error message is static which is set from _user_access_timeslot_logout_user() function. You can make it dynamic using system confing form on the 'admin/config/people/user-access-timeslot' page itself. so that admin user can set this message.

2) Error messages on config form.

When I set invalid time for Wednesday (like for morning - from 5:00 AM to 3:00 AM) which is invalid. It shows below error message.

Wednesday start date can't be after end date.

In this error message its not clear that for which time slot it showing validation. Whether its for morning or afternoon. You can include specific time slot (morning or afternoon) in error messages.

Thanks

@Nixou,

good to see your module.

finding:
you need one more validation for admin configuration page, where admin should not enter same Time for active/inactive.
that can lead your code to Deadlock. refer[user_access_timeslot.png]

Optional:
As a first time visitor you can add help text/prefilled text refer:[user_access_timeslot_help_text.png]

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 / No: Does not follow] the licensing requirements.

3rd party assets/code

[Yes: Follows ] the guidelines for 3rd party assets/code.

README.txt/README.md

[Yes: Follows] the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

[Yes: Follows] the guidelines for project length and complexity.

Secure code

[Yes: Meets the security requirements.]

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

1. (*)When I clone the module then it was showing module name as "2757621" rather than "user_access_timeslot"
2. (*)Remove $form['#submit'][] = 'user_access_timeslot_settings_form_submit'; in line number 188 as well as validation because automatically submit and validate handled
3. (*)In hook_init if we enable cache page in "configuration>development section" then hook_init not called.then you expired condition not check
4. (*)Can you add extra feature to selected different time zone rather than default timezone, so that admin can add different inactive time for different timezone of role user
   Like Eg. Multi select functionality.

Git clone failed for https://git.drupal.org/sandbox/Nixou/2651698.git while invoking http://pareview.sh/pareview/httpsgitdrupalorgsandboxNixou2651698git

Git clone failed. Aborting.

I'm a robot and this is an automated message from Project Applications Scraper.

Module looks good and working for me. I think we don't have any blocker points. So i am marking as RTBC.

RTBC +1

Useful module. I see no blockers now, just a few comments.

When setting a timeslot, the final schedule is excluded from the allowed period.
e.g.: In case the admin sets

Authorize access between Monday from 7:00pm to 8:00pm

The user will be only able to login until 7:59pm. If the user tries to login at 8:00pm, he will see the error message. (same for expiration).
In my opinion it should be either documented or changed to only block or expire in the next minute.

user_access_timeslot.api.php - not sure why you have some custom code inside the hook definitions. They are not necessary and may be confusing, I would suggest you to leave just the definitions and document the use.

The function _user_access_timeslot_get_expiration_date() is a bit confusing to read, but not an issue.

Thanks for your contribution, Léo!

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:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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.