[Bug 5031] New: CSV export profile form doesn't indicate that profile marcfields are required
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Summary: CSV export profile form doesn't indicate that profile marcfields are required Change sponsored?: --- Product: Koha Version: rel_3_2 Platform: All OS/Version: All Status: NEW Severity: normal Priority: P5 Component: Tools AssignedTo: gmcharlt@gmail.com ReportedBy: oleonard@myacpl.org QAContact: koha-bugs@lists.koha-community.org Estimated Hours: 0.0 If you try to save a CSV export profile without filling in profile marcfields, the page refreshes without saving any data and without showing any warnings. The form should at least indicate that it is a required field. A proper fix would display warnings if the profile was not saved because of missing data. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|rel_3_2 |master --- Comment #1 from Owen Leonard <oleonard@myacpl.org> --- In fact the script doesn't have any error handling at all. It will process inserts, updates, and deletes if all the required data is submitted: if ($profile_name && $profile_content && $action) { ... But if any of those doesn't exist nothing happens. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jcamins@cpbibliography.com --- Comment #2 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- *** Bug 9960 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 20966 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20966&action=edit Bug 5031: CSV profiles: content fields are required Some fields are required on editing/adding csv profiles. Test plan: - add a new csv profile (tools/csv-profiles.pl) without any value. - save => error on empty fields. - fill the "marc" content, select type="sql", fill the csv name input. - save => error on sql content field. - fill the sql content - save => the csv profile is saved. - retry for the edit form. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |jonathan.druart@biblibre.co | |m Patch complexity|--- |Trivial patch Depends on| |10853 Assignee|gmcharlt@gmail.com |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 I'm just a bot <gitbot@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |gitbot@bugs.koha-community. | |org --- Comment #4 from I'm just a bot <gitbot@bugs.koha-community.org> --- Applying: Bug 5031: CSV profiles: content fields are required Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/en/modules/tools/csv-profiles.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/csv-profiles.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/tools/csv-profiles.tt Patch failed at 0001 Bug 5031: CSV profiles: content fields are required The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #5 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to I'm just a bot from comment #4) Depend on bug 10853 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20966|0 |1 is obsolete| | --- Comment #6 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 21303 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21303&action=edit [SIGNED-OFF] Bug 5031: CSV profiles: content fields are required Some fields are required on editing/adding csv profiles. Test plan: - add a new csv profile (tools/csv-profiles.pl) without any value. - save => error on empty fields. - fill the "marc" content, select type="sql", fill the csv name input. - save => error on sql content field. - fill the sql content - save => the csv profile is saved. - retry for the edit form. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: On top of both patches for Bug 10853 Works very well, now we get an alert with empty field Saves correctly MARC and SQL profiles Solved minor conflict introduced by tab followup on Bug 10853 Again small tab errors corrected in followup -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 --- Comment #7 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 21304 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21304&action=edit Bug 5031 - followup to correct tabs Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |bgkriegel@gmail.com -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion CC| |katrin.fischer@bsz-bw.de, | |oleonard@myacpl.org --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Jonathan, moving on from bug 10853 to this one. The highlighting of the required fields is a good addition, but the formatting seems a bit non-standard. Maybe check Owen's work on bug 6209? Putting 'In Discussion' and adding Owen in CC in hope for some feedback. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 --- Comment #9 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #8)
Hi Jonathan,
moving on from bug 10853 to this one. The highlighting of the required fields is a good addition, but the formatting seems a bit non-standard. Maybe check Owen's work on bug 6209?
Katrin, what do you consider as non-standard? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- It looks really nice, but not like in other places of Koha :) But maybe Owen could take a look later? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #10)
It looks really nice, but not like in other places of Koha :) But maybe Owen could take a look later?
I used the "required" attribute and the "required" class. The attribute is a new one (html5) and the required class is the way to do, I think. I switch back to Signoff: since bug 10853 is pushed to master, I think this one should be in 3.14 I will quickly submit another patch if Owen considers something is wrong. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Yes, that's ok. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21303|0 |1 is obsolete| | Attachment #21304|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 23257 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23257&action=edit [PASSED QA] Bug 5031: CSV profiles: content fields are required Some fields are required on editing/adding csv profiles. Test plan: - add a new csv profile (tools/csv-profiles.pl) without any value. - save => error on empty fields. - fill the "marc" content, select type="sql", fill the csv name input. - save => error on sql content field. - fill the sql content - save => the csv profile is saved. - retry for the edit form. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Comment: On top of both patches for Bug 10853 Works very well, now we get an alert with empty field Saves correctly MARC and SQL profiles Solved minor conflict introduced by tab followup on Bug 10853 Again small tab errors corrected in followup Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described, looks a bit 'non standard' for Koha, but improves usability of the form. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 23258 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23258&action=edit [PASSED QA] Bug 5031 - followup to correct tabs Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tiny QA follow-up corrcting whitespace to match coding guidelines. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |gmcharlt@gmail.com --- Comment #15 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. Thanks, Jonathan! -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |11333 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 --- Comment #16 from Galen Charlton <gmcharlt@gmail.com> --- (In reply to Jonathan Druart from comment #11)
(In reply to Katrin Fischer from comment #10)
It looks really nice, but not like in other places of Koha :) But maybe Owen could take a look later?
I used the "required" attribute and the "required" class. The attribute is a new one (html5) and the required class is the way to do, I think. I switch back to Signoff: since bug 10853 is pushed to master, I think this one should be in 3.14 I will quickly submit another patch if Owen considers something is wrong.
I've opened bug 11333 and attached a patch that makes the two forms on this page use the jQuery validation plugin, which should address some of Katrin's concerns. The way that this form handles editing existing profiles is decidedly not consistent with the rest of Koha, but that would be the topic of another bug. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5031 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable CC| |fridolyn.somers@biblibre.co | |m --- Comment #17 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Those patches has been pushed to 3.14.x, will be in 3.14.1. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org