| Summary: | CSV export profile form doesn't indicate that profile marcfields are required | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Owen Leonard <oleonard> |
| Component: | Tools | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Bugs List <koha-bugs> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | bgkriegel, fridolin.somers, gitbot, gmcharlt, jcamins, jonathan.druart, oleonard |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 10853 | ||
| Bug Blocks: | 11333 | ||
| Attachments: |
Bug 5031: CSV profiles: content fields are required
[SIGNED-OFF] Bug 5031: CSV profiles: content fields are required Bug 5031 - followup to correct tabs [PASSED QA] Bug 5031: CSV profiles: content fields are required [PASSED QA] Bug 5031 - followup to correct tabs |
||
|
Description
Owen Leonard
2010-07-16 17:15:01 UTC
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.
*** Bug 9960 has been marked as a duplicate of this bug. *** Created attachment 20966 [details] [review] 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. 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". (In reply to I'm just a bot from comment #4) Depend on bug 10853 Created attachment 21303 [details] [review] [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 Created attachment 21304 [details] [review] Bug 5031 - followup to correct tabs Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> 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. (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? It looks really nice, but not like in other places of Koha :) But maybe Owen could take a look later? (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. Yes, that's ok. Created attachment 23257 [details] [review] [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. Created attachment 23258 [details] [review] [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. Pushed to master. Thanks, Jonathan! (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. Those patches has been pushed to 3.14.x, will be in 3.14.1. |