Closed (fixed)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
15 Mar 2016 at 12:11 UTC
Updated:
4 Apr 2017 at 09:04 UTC
Jump to comment: Most recent
Comments
Comment #2
bechtold commentedI will install and test it.
While you wait you could fix the code style issues ;-)
http://pareview.sh/pareview/httpgitdrupalorgsandboxsgurlt2685813git
Cheers
Comment #3
bechtold commentedIt is working.
But I was a bit confused what error message is replaced and it looks like it is only the required message.
I'm not even sure if there would be any other error messages. What happens if I enable this on a field with other validation like the file field or the email field?
If it is only replacing the required error maybe you should rename it to "Required Error Message" and move it next to the required checkbox. Maybe even hide it with state if required is not checked.
Other than that good idea.
Cheers
bechtold
Comment #4
sgurlt commentedYou were right, it was a bit unclear what this module does according to required error messages / additional error messages.
I added a config backend where it is now possible to select what the module should actually do to which entity type -> bundle. You can chose if you want to override all field error message including additional validation like for example when a mail address is not valid, or you can select if you just want to override the required error message.
In addition to that i fixed the coding standard issues ;)
Comment #5
PA robot commentedWe 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.
Comment #6
bechtold commentedGood idea.
Would it be possible to move the settings to the field settings?
First of all I might want to use different settings on different fields on the same entity, I believe that is currently not possible.
Also I find the config page quite confusing (I have 13 entities with multiple bundles) and I didn't find it. I was instantly looking on the field settings and was wondering if the update didn't work.
Then I'm not sure if this way it is possible to integrate into a nice feature workflow. When I have one feature per entity, all the fields error settings should automatically be included into that feature. And I'm not sure if it is splittable at the moment anyways or if all the settings needed to go into one feature.
I still get quite a lot errors: http://pareview.sh/pareview/httpgitdrupalorgsandboxsgurlt2685813git
Comment #7
sgurlt commented"Would it be possible to move the settings to the field settings?
First of all I might want to use different settings on different fields on the same entity, I believe that is currently not possible."
Sadly not since I add a validator to the hole form and not a specific field. By adding the validator to a specific field, I would not be able to override the error messages.
"Then I'm not sure if this way it is possible to integrate into a nice feature workflow. When I have one feature per entity, all the fields error settings should automatically be included into that feature. And I'm not sure if it is splittable at the moment anyways or if all the settings needed to go into one feature."
For the beginning i would like to keep all settings in one feature. I have seen quite a lot modules doing it in this way. But I think it would be possible to split it up in the future if there is really a need for that.
I am going to review the errors later that day :)
Comment #8
sgurlt commentedErrors are now also fixed:
http://pareview.sh/pareview/httpgitdrupalorgsandboxsgurlt2685813git
Comment #9
gisleTo get people to review your application, you need to set its state to "Needs review". Please see project applicaton workflow.
Comment #10
visabhishek commentedAdding clone url:
Comment #11
sriharsha.uppuluri commentedAutomated Review
Please fix reported issue on https://pareview.sh/node/208
Manual Review
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.
Comment #12
poojasharmaece commentedPlease fix issue reported on https://pareview.sh/node/208
Make sure delete all variables used in module under hook_uninstall();
Example :
variable_get($form['#entity_type'] . '_' . $form['#bundle'] . '_error_message_settings');Comment #13
klausiMinor coding standard errors and deleting variables are not application blockers. Anything else that you found or should this stay RTBC?
Comment #14
poojasharmaece commentedHi Klausi,
I set it "Needs Works" as per point "Not deleting setting variables" on https://www.drupal.org/node/532400
Example review issues that warrant going back to Needs work
Is i am missing anything ? It will really help me for other project review.
Comment #15
avpadernoSince the purpose of this queue is changed, I am not sure forgetting to delete a persistent variable is a blocker anymore.
Comment #16
avpadernoThank you for your contribution!
I will update your account so you can opt into security advisory coverage. See Goodbye Project Applications, Hello Security Advisory Opt-in to understand what that means, and what is different from the previous project application purpose.
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 stay involved!
Thank you, 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 go the dedicated reviewer(s) as well.
Comment #17
klausiAssigning credits.