[D7] Required Fields

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

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.

You are currently working on 7.x-1.0 branch please have a look at Release naming conventions and change accordingly.

Coder review shows one critical warning.

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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

Hello there,
A manual review on the project.

First, the project page description is too short. May be you can explain what the module does, and also provide a screenshot ?

Code seems OK to me. However, just a suggestion - do you think it would be easier for site admins if you put the block in Administer > Structure > Content types ?
You can also check if the field collection module exists in the hook_menu implementation to prevent your module from registering the path if field_collection module is not available.
When enabling/disabling a module, menus will be rebuilt automatically so there won't be a problem with caches either.

Good luck!

Please add all your reviews to the issue summary, so that we can track them. When finishing your review comment also set the issue status either to "needs work" (you found some problems with the project) or "reviewed & tested by the community" (you found no major flaws).

manual review:

1. you could improve the structure of your project page a bit, see https://drupal.org/node/997024
2. required_fields_menu(): actually you could support all entity types, right? entity_get_info() gives you all types and you could load all field info for them?
3. required_fields_collection_configuration(): the module_exists() check should be in hook_menu(), so that the link does not show up if there is no field_collection module.
4. required_fields_configuration(): why do you have to clear the field info cache every time your config page is loaded? That will just slow your site down, right?
5. required_fields_ajax_callback() do not use db_select() and db_update() to change field information! There might be modules that want to react on those changes, so you must use the appropriate field API functions.
6. required_fields_prepare_admin_form(): this is vulnerable to XSS exploits. If I enter <script>alert('XSS');</script> as field label I will get a nasty javascript popup. Field labels are user supplied text and therefore need to be sanitized before printing to HTML. Make sure to read https://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.

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. Feel free to reopen if you are still working on this application (see also the project application workflow).

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

Removed copies of automated review comments.

manual review:

1. info file: Why do you need the CSS file on every single page request? I think you should only add it to your admin page or wherever you need it.
2. required_fields_collection_configuration(): the module_exists() call here is now obsolete, right?

But that are not critical application blockers, otherwise looks RTBC to me.

Assigning to cweagans as he might have time to take a final look at this.

Hello there,
I care about this module.

1. As Klausi said, info in the CSS file does not need to be loaded in each page, but I suggest here you can use the theme of table output, so that the required_fields.css is not needed, the page more beautiful than this.
2. line 127, $instance_info['required'] == 1 change to $instance_info['required'] == TRUE is better i think.
3. I saw in the form, there is a parameter of $count transfer, its role seems to form #name submitted does not repeat, I think you can use the field ID (is field_config_instance ID, not field_id).
4. This is a user experience module, if only for the administrator to use, the effect is not obvious, recommended opening permissions to the user role, can have more play.

no objections for more than a week, so ...

Thanks for your contribution, silambarasan!

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.