Module Description

Required On Publish will let you save any node in unpublished state even if there are some required fields left unfilled. Those required fields will act only when node is saved in published state, i.e. if some required fields are still left empty while publishing the node then it will not allow until all the required fields are non-empty.

Project Page:

https://www.drupal.org/sandbox/abhishek_pareek/2549931

Git Clone:

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/abhishek_pareek/2549931.git required_on_publish
cd required_on_publish

PAReview:

http://pareview.sh/pareview/httpgitdrupalorgsandboxabhishekpareek2549931git

Manual reviews of other project:

1. https://www.drupal.org/node/2676036#comment-10906678
2. https://www.drupal.org/node/2677542#comment-10905490
3. https://www.drupal.org/node/2678258#comment-10909880

Comments

abhishek.pareek created an issue. See original summary.

PA robot’s picture

Issue summary: View changes

Fixed the git clone URL in the issue summary for non-maintainer users.

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.

pankajsachdeva’s picture

Automated Review

No issues found.

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] 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
Everthing looking good.

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

pankajsachdeva’s picture

Status: Active » Reviewed & tested by the community
jitujain’s picture

Status: Reviewed & tested by the community » Needs review

Nodes on the "admin/config/content/list_required" this page are visible after filling the all required fields. This page shows all the unpublished nodes, you have to show only those nodes which are unpublished and have the required field.

If there is no records to show in table then display a message instead showing header of table.
install file should be there.

jitujain’s picture

Status: Needs review » Needs work
abhishek.pareek’s picture

Status: Needs work » Needs review

Nodes on the "admin/config/content/list_required" this page are visible after filling the all required fields. This page shows all the unpublished nodes, you have to show only those nodes which are unpublished and have the required field.

If there is no records to show in table then display a message instead showing header of table.
install file should be there.

Fixed all the issues stated above.

basant’s picture

I ran automated review on your code and got following error :
PHP Fatal error: Can't use function return value in write context in ./required_on_publish.module on line 148
Errors parsing ./required_on_publish.module

ziomizar’s picture

Hi abhishek.pareek,

On line 136 and 95 and for all the other strings you should use t() function.

abhishek.pareek’s picture

I ran automated review on your code and got following error :
PHP Fatal error: Can't use function return value in write context in ./required_on_publish.module on line 148
Errors parsing ./required_on_publish.module

Hi basant, well i corrected that one. Thanks for that !

On line 136 and 95 and for all the other strings you should use t() function.

Thanks ziomizar, I wrapped those strings in t() function.

abhishek.pareek’s picture

Issue summary: View changes
saraswathi ravikumar’s picture

Hi abhishek.pareek,

Please use t() function for the return statements. Refer to line number 187.

EX : return t('You are not allowed to access this page.');

abhishek.pareek’s picture

Hey sarah ravikumar,
I applied t() upon that string, thanks for pointing out that.

Is there anything else in this module, which you would like to suggest me ?

perennial.sky’s picture

Status: Needs review » Needs work

Hi Abhishek

I tried your module, and I impressed with this

I would point out some issue that I found in your module

1. When I tick Required validation only when publishing setting, It mean field is required but I can't see any required star mark on that field, Add that so that user can understand that which field is required

2. In module line number 172, use t() function

3. Please change module package name in info file

4. There is menu named admin/config/content/list_required it shows list of empty field, I think it should come under report page not in configuration

5. You created the menu but you define its callback in the module file itself it must be required to write any menu callback in a different file.

6. You are using prefix and suffix instead of that you can use container type form element,

7. use annotation instead of using variable directly in l() function

PA robot’s picture

Status: Needs work » Closed (won't fix)

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.

abhishek.pareek’s picture

Hi Akash,

Thanks for reviewing my code :)

2. In module line number 172, use t() function

That is done.

3. Please change module package name in info file

Well I removed the pakage from info file, that will put my module in others category, that is fine.

4. There is menu named admin/config/content/list_required it shows list of empty field, I think it should come under report page not in configuration

Well Yes, that was not appropriate earlier, so I changed it to 'admin/reports/list_required' I guess that is fine now.

6. You are using prefix and suffix instead of that you can use container type form element,

Well I don't have many fields and so I don't want them in a container field, but I needed form inside a div tag so I used prefix and suffix

7. use annotation instead of using variable directly in l() function

OK, so I put title in t() and used annotation for title variable.

Now comes the most important point

1. When I tick Required validation only when publishing setting, It mean field is required but I can't see any required star mark on that field, Add that so that user can understand that which field is required

Well that is a tricky part and I am not able to solve it :) Well it can be possible through overriding theme function, so I will leave that for end user if they really want that asterisk, by the way that asterisk is not really required in my case as I am allowing user to save node in unpublished state without caring about required. So that is actually an ambiguous state. But if someone comes up with a good solution of it, then I would love to provide that asterisk as an optional part.

abhishek.pareek’s picture

Status: Closed (won't fix) » Needs review
PA robot’s picture

Status: Needs review » Closed (duplicate)
Multiple Applications
It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2751523

Project 2: https://www.drupal.org/node/2687537

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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

abhishek.pareek’s picture

Status: Closed (duplicate) » Needs review

This is not a duplicate application, although those manual reviews were, so I have changed those manual reviews in another application of mine.

abhishek.pareek’s picture

Issue tags: +PAreview: review bonus
PA robot’s picture

Multiple Applications
It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2687537

Project 2: https://www.drupal.org/node/2751523

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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

abhishek.pareek’s picture

Status: Needs review » Postponed
PA robot’s picture

Status: Postponed » Closed (duplicate)
Multiple Applications
It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2751523

Project 2: https://www.drupal.org/node/2687537

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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

avpaderno’s picture