Instead of showing a standard "404 Page not found", this module
shows a childfocus missing poster

https://drupal.org/sandbox/paulvb/2071587

git clone http://git.drupal.org/sandbox/paulvb/2071587.git not_found

CommentFileSizeAuthor
#6 not_found-ajax_form-2074175-5.patch2.72 KBkasalla

Comments

chester_martin’s picture

Hi,

I hadn't heard about notfound.org before but love the idea and your effort for a Drupal integration.

An automated test shows some coding standards issues that needs to be resolved:
http://pareview.sh/pareview/httpgitdrupalorgsandboxpaulvb2071587git

Also I have a few suggestions:
- It might be better to integrate this with the standard 404/403 settings fieldset on admin/config/system/site-information using hook_form_alter().

- Your project page and the description on this issue thread wouldn't suffer from some basic info about notfound.org.

- My personal opinon is also that I would like a way to specify a message that describes what the user is looking at on the 404-page.

Otherwise, I tested on a live site and everything worked as aspected.

Martin

chester_martin’s picture

Status: Needs review » Needs work
kasalla’s picture

manual review:

not_found.install
------------------------------
on line 11 you set the variable value of "site_404" to "not_found". But in hook_uninstall you check if the variable has the value "notfound404" and then delete it (maybe set it back to default?). I think it should be "not_found" too.

not_found.module
------------------------------
Find no problems, but:

From the Project application checklist:

2.3 Ensure your project contains a minimum of handwritten code.
We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed, that means we can't make sure you're able to write secure code and use Drupal's APIs correctly.
If this is the case, we can't approve you as git vetted user (giving you the permission to create full projects). Either we can see that you have the needed knowledge by other contributions within the drupal community or we can promote it to a full project manually.

Kind regards
kasalla

paulvb’s picture

@Chester_martin

Thanks for the feedback. I integrated your suggestions in the project.

- http://pareview.sh/pareview/httpgitdrupalorgsandboxpaulvb2071587git, the test is without errors now.
- I liked the suggestion about putting the config under site-information. It will be better for useability so I changed that also
- I added a description to my project page which I also added to my README
- I added a description field to the config that will be printed above the config

@kasalla

Thanks for your feedback as well.

- I corrected the bug in the .install
- Since i won't be writing code just to get to 120 I think it would be better to promote the project manually.

chester_martin’s picture

Hi paulvb,

Great work! looks awesome. One last suggestion from me:

To make it even more sleek, you might consider the possibility to enhance the settings form with some ajax. Instead of typing in "not_found" in the textfield you could put a checkbox somewhere "Use notfound.org" (or something). When checking that box the original textfield is hidden and your notfound-specific fields appear.

As I said, that's just a suggestion, but probably it will get you closer to the 120 lines at the same time;-)

Good luck!

kasalla’s picture

Status: Needs work » Needs review
StatusFileSize
new2.72 KB

Created a small patch for the AJAX form, maybe you want to use it.

edit:// I made a small mistake at line 53.. =/ shame on me

PA robot’s picture

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.

bohart’s picture

Status: Needs review » Closed (won't fix)
Duplication
This sounds like a feature that should live in the existing project (see https://drupal.org/project/childfocus-notfound). Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the childfocus-notfound issue queue to discuss what you need. You should also get in contact with the maintainer (@dimitrivh) to offer your help to move the project forward. If you cannot reach the maintainer please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

Regards,
bohart.