[D7] Office Hours Extra

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxphoehne2329615git

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.

I love the idea as it improves the UX of office hour module.

I have a quick look on the code and found you use document ready rather then attach behaviours in js/timetable.js.

/**
 * @file
 * Timetable widget logic.
 */

(function ($) {
  $(document).ready(function () {

    $(".timetable-wrapper").each(function (index) {

I recommend you to change above into below which would make other developer to have better control :

/**
 * @file
 * Timetable widget logic.
 */

(function ($) {
  Drupal.behaviors.officeHourExtras = {
    attach: function(context, settings) {
      $(".timetable-wrapper").each(function (index) {

Automated Review found a couple of errors (Non-critical, but best practices)
http://pareview.sh/pareview/httpgitdrupalorgsandboxphoehne2329615git

Individual user account Follows.
No duplication: Does not cause.
Master Branch: Follows.
Licensing: Follows
3rd party assets/code: Follows.
README.txt/README.md: Does not follow.
Code long/complex enough for review: Follows.
Secure code: Meets the security requirements.
Coding style & Drupal API usage:
*Use the template for README.txt https://www.drupal.org/node/2181737

Additional Comments
I see you are now using Drupal.behaviors in javascript (Awesome)
Still using document ready in office_hours_extra.admin.inc ln 74

The css class naming is not unique to the project, this could cause conflicts. I'd suggest adding ohe to css class names
Which version of jQuery is required? Add to readme?

installed module, went to admin/config/date/timetable, got an error:
Notice: Undefined variable: rows in office_hours_extra_list() (line 177 of C:\work\office_hours_extra\office_hours_extra.admin.inc).

added:
i've just realized, seems like this error happened when i tried to add new profile without choosing any dates.

Works great, A couple of minor improvements for usability

1. When user is selecting the time slots whether to "Add" or "Remove" , the appropriate button should be highlighted. Although the mouse cursor has that indication, having it on the button would help
2. Saving a profile without choosing a date redirects user back to profile page without telling the user what happened. You should give a message indicating the reason

Automated Review:
Passed Coder module rests

Manual Review

Individual user account : Yes
No duplication : Yes
Master Branch : Yes
Licensing: Yes
3rd party assets/code : Yes
README.txt/README.md: Yes
Code long/complex enough for review: Yes
Secure code: yes

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

• Codespell has found some spelling errors in your code.
  

  ./README.txt:20: dont  ==> don't
  

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

manual review:

1. why can't we integrate your features into the core office hours project? Did you open an issue there to integrate your stuff without adding another module that is hard to find?
2. exception_cal.js: months and weekday abbreviations should be run through Drupal.t() for translation.
3. office_hours_extra_field_widget_form(): doc block: do not duplicate the hook docs, noting that this implements a hook is enough.
4. _office_hours_extra_field_widget_multidatespicker_form(): doc block is wrong, this is not a hook.
5. _office_hours_extra_field_widget_multidatespicker_form(): this is vulnerable to XSS exploits. If I enter <script>alert('XSS');</script> as field label for an office hours field I will get a nasty javascript popup on the edit form of an entity. You need to sanitize user provided text before printing, make sure to read https://www.drupal.org/node/28984 again. And please don't remove the security tag, we keep that for statistics and to show examples of security problems. This is mitigated by the fact that an attacker must have the permission to administer fields, but currently that is already possible with the relatively innocent "administer taxonomy" permission in Drupal 7.

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

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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