The Job-a-matic module uses SimplyHired's XML API to display job postings from their database directly within your Drupal site.

Features:

* Single job search page without using a node.
* Block display of the latest jobs
* Fully themeable output for collective jobs search results and individual job search result.

This project is for Drupal 7 only and will not be back ported.

[Edit - corrected sandbox project page URL]
Project page: http://drupal.org/sandbox/r0nn1ef/1499502
Git repo: git clone http://git.drupal.org/sandbox/r0nn1ef/1499502.git simply_hired_job_a_matic

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alfababy’s picture

Status: Active » Needs work

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

./simply_hired_jobamatic.features.inc: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming

function jobamatic_custom_search_form($form, &$form_state) {
Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
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.

EDIT: removed long pareview.sh output.

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

r0nn1ef’s picture

Status: Needs work » Needs review
FileSize
381.54 KB

Fixed all issues reported by PAReview (ventral.org)

monymirza’s picture

Status: Needs review » Needs work

Given Project page link not working.

r0nn1ef’s picture

Status: Needs work » Needs review
jgalletta’s picture

Status: Needs review » Needs work

Hi,

I did a little Manual review:

  • CHANGELOG.txt is empty, thus useless.
  • simply_hired_jobamatic.install

l4: Drupal comments should follow proper grammar rules, and thus contain ponctuation
l11: see drupal function st() http://api.drupal.org/api/drupal/includes!install.inc/function/st/7
I'm not sure a t() function in the variable table is a good thing though

  • simply_hired_jobamatic.module

l81: Why don't you use module_load_include() function?
l162: add a space character before and after = sign
l219: is it necessary?

  • simply_hired_jobamatic.features.inc

l0: filename could be mistaken with a features file from Features module
l72: Add t() so each option can be translatable.
l87: Add t() so each option can be translatable.

  • simply_hired_jobamatic.features.inc

l66: Add t() so each option can be translatable.
l81: Add t() so each option can be translatable.

  • simply_hired_jobamatic_job.tpl.php

l45: Use a t() to translate "Location"
l45: Use a t() to translate "Job description"

  • simply_hired_jobamatic_attribution.tpl.php

selveral lines: couldn't you use css in css file instead of inline style code?

  • jobamatic.js

l1: Comment file purpose
l14 & 31: add a space character after the 'if'
l20 & 39: javascript comments should follow same Drupal comments guidelines as anywhere else, so Capitalized letter and ending dot.
l30: comment function

  • latest-jobs.js

l1: Comment file purpose
l9: comment function

r0nn1ef’s picture

Status: Needs work » Needs review
FileSize
86.94 KB

Thanks for the look at the code base. Below are actions/in-actions taken on each item.

* Changelog.txt - added initial changelog

* simply_hired_jobamatic.install

|4 - added full stop in comments
|11 - this is correct as per community docs
(@see http://drupal.org/node/322731)

* simply_hired_jobamatic.module

|81 - fixed
|162 - fixed
|219 - Not really needed, but since Drupal has moved away from
using an underscore to denote private functions (outside of objects),
i used the the @access tag so developers would know this function
is meant for use only by this module.

* simply_hired_jobamatic.features.inc

|0 - renamed file to eliminate the possible confusion
|72 - fixed
|87 - fixed

* simply_hired_jobamatic_job.tpl.php

|45 - fixed
|46 - fixed

* simply_hired_jobamatic_attribution.tpl.php

Attribution is exactly as required by the terms of service (see attached screenshot)

* jobamatic.js

|1 - fixed
|14 & 31 - fixed
|20 & 39 - fixed
|30 - fixed

* latest-jobs.js

|1 - fixed
|9 - fixed

adnasa’s picture

This looks good.
good luck!

klausi’s picture

We 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 :-)

vladimir-m’s picture

Status: Needs review » Needs work

Hello r0nn1ef,

Thank you for great module.

1. Please review your module from coder module and see the automated code review http://ventral.org/pareview/httpgitdrupalorgsandboxr0nn1ef1499502git

Manual review:
2. In file: simply_hired_jobamatic.module at line: 93 variable $show_pager was initialized but it is not used in simply_hired_jobamatic_do_job_search() function.

r0nn1ef’s picture

Status: Needs work » Needs review

Thank you for your feedback. Ventral.org must have updated their rules because in comment #2 above, I attached a screenshot of where i ran my code through pareview and it didn't find anything.

In any case, I have re-run it through pareview and fixed all the problems as well as made use of the un-used variable.

darrenwh’s picture

Status: Needs review » Needs work

Hi,

Line 215 in module
You dont have an action to do as the default so you can omitt this.

simply_hired_jobamatic_render_page(), $data not declared as an array.
simply_hired_jobamatic_api:
call() method commented as private though it is actually protected
getCode() $code not defined
search() use a more descriptive var name instead of $c, ditto with call()
Look to simplify the parseXML() method, add comments, perhaps break out into new class + sub methods

Darren

r0nn1ef’s picture

Status: Needs work » Needs review

I've updated the module with the following from #11.

* removed default case from switch statement since there is no default action
* declared $data as array
* changed comment for call() so that it properly reflects the correct access
* changed to use proper variable from $code to $this->code in getCode()
* updated variables in search() and call() to have more descriptive names
* added more comments to parseXML() -- evaluating the other options mentioned.

chris.smith’s picture

Status: Needs review » Needs work
FileSize
6.71 KB
  • If there are no latest jobs in the area specified, something should notify the user such as 'No Results' instead of having a blank box with no text.
  • No matter which city/state or postal code is put into the location I still receive 'No Results'. Tried altering the location in the configuration page and advanced search options and both did not work.
  • Make 'Jobs by Simply Hired' a full link.

Attached image: Screenshot taken after entering Los Angeles, California into the location

r0nn1ef’s picture

Status: Needs work » Postponed (maintainer needs more info)
FileSize
485.23 KB

Could not reproduce the problems using the limited information submitted by #13. I used the city/state combination mentioned above on my demo site at www.ronferguson.net/tools/jobs and got a full result set back.

Please submit screenshot of admin configuration options and specific actions/path on the site that you used so I can determine if there is a configuration problem that I need to account for in the error checking.

chris.smith’s picture

Status: Postponed (maintainer needs more info) » Needs review
FileSize
37.7 KB
37.19 KB

I installed the module using the clone you provided. I then customized the configuration page so that it had my job-a-matic site linked to it. When I attempted to search I still got 'No Result'. Please view the screenshots I have provided, hope they help.

r0nn1ef’s picture

I looked at the screenshots from #15 and I found the problem. The jobamatic parameters for publisher ID and jobamatic domain are not correctly entered. I've attached an updated version of screenshot-3.png highlighting the problems. I'm also attaching a screenshot of the Jobamatic portal XML API page highlighting the area of that page where those parameters can be found.

I will, however, add more documentation to the settings for so that the user knows where to find these parameters. Simple fix for your problem and good catch to show me there needs to be more end-user info presented.

r0nn1ef’s picture

I've just updated the module to include field descriptions for the Publisher ID and Jobamatic domain fields for clarity as well as added a validation function for the Publisher ID.

glesage’s picture

Status: Needs review » Reviewed & tested by the community

Tested and working

kscheirer’s picture

  • Remove .gitignore
  • There are some new errors on http://ventral.org/pareview/httpgitdrupalorgsandboxr0nn1ef1499502git (they do update the standards periodically)
  • Especially the namespace issues should be resolved
  • I'm going to ask for a second opinion about the module shortname simply_hired_jobamatic and using the string 'simply_hired' in some of the urls. I'm not sure if Drupal has a policy about this.

Otherwise, the code is very nice.

----
Top Shelf Modules - Enterprise modules from the community for the community.

kscheirer’s picture

Title: Simply Hired Job-a-matic » [D7] Simply Hired Job-a-matic
kscheirer’s picture

Status: Reviewed & tested by the community » Needs work

Greggles had the following concern at https://groups.drupal.org/node/157669#comment-943538 that the required attribution block:

I would want it clearly stated in the project page and README.txt that this module adds links back to the site.

If you could make the attribution line optional (can default to enabled), I think that would also be a solution. Otherwise this looks ready to go!

----
Top Shelf Modules - Enterprise modules from the community for the community.

r0nn1ef’s picture

Status: Needs work » Needs review
FileSize
259.18 KB

I actually anticipated this when the module was first developed and therefore initially built in a way to disable the attribution in the module config. See the attached screenshot for it's specific location in the config. I have also updated the project page and the README.txt file with a notice of the requirements as well.

kscheirer’s picture

Status: Needs review » Fixed

Works for me, thanks for the additional information. It was my mistake to think it was required. There's some issues to fix at http://ventral.org/pareview/httpgitdrupalorgsandboxr0nn1ef1499502git.

Thanks for your contribution, r0nn1ef!

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.

----
Top Shelf Modules - Crafted, Curated, Contributed.

r0nn1ef’s picture

Status: Fixed » Closed (fixed)

I've fixed the outstanding issues reported by ventral.org. Thanks for giving me access. It's been, at times, frustrating getting to this point. But I understand why and I believe this process is what will keep the Drupal community in better shape than most other open-source projects. Thanks to everyone who helped with the review process.

r0nn1ef’s picture

Issue summary: View changes

corrected sandbox project page URL