---- Reported by cmurdock@ccfls.org 2010-03-30 16:57:46 ---- The only way to change an existing issuing rule is to re-enter all the values for it as if you were adding a new rule. There should probably be a simpler way to edit an existing issuing rule, so you can more easily change a single value if you want to. --- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:26 UTC --- This bug was previously known as _bug_ 4354 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=4354 Actual time not defined. Setting to 0.0 Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one.
Created attachment 2182 [details] [review] Proposed fix This patch adds javascript which pulls the values from the chosen row of data and populates the last row's form with that data. Some changes have been made to the markup to better accommodate this. Possibly missing is some kind of visual feedback to the user that the edit action is taking place in a different row from where they clicked. Comments are especially welcomed on this aspect of the interaction.
+1 I've tested it. It works and is useful.
Created attachment 2932 [details] [review] Revised for current HEAD
Created attachment 2946 [details] [review] Revised for current HEAD
(In reply to comment #4) > Created attachment 2946 [details] [review] > Revised for current HEAD Hie, Nice patch, it saves a lot of time. I just had a problem after merging : When clicking on "edit", the submit button value changed to "Edit". It javascript, you should modify to apply edition only on text input : L16 : $("#default-circulation-rules tr:last td:eq("+i+") input[type=text]").val(itm); Regards,
This has already been done in one of our patches waiting to be pushed. via jedit
i thas also be done in our BibLibre-cat-circ branch. in smart-rules, each cell of the table can be edited and saved independantly. Very convenient, libraries loves it !
This bug is mentioned in: Proposed fix for Bug 4354, Cannot edit existing issuing rules http://lists.koha-community.org/pipermail/koha-patches/2011-January/013519.html Proposed fix for Bug 4354, Cannot edit existing issuing rules http://lists.koha-community.org/pipermail/koha-patches/2011-January/013512.html
*** This bug has been marked as a duplicate of bug 5872 ***
*** Bug 8363 has been marked as a duplicate of this bug. ***
Created attachment 14622 [details] [review] Bug 4354: Cannot edit existing issuing rules This patch adds javascript which pulls the values from the chosen row of data and populates the last row's form with that data. Some changes have been made to the markup to better accommodate this. Possibly missing is some kind of visual feedback to the user that the edit action is taking place in a different row from where they clicked. Comments are especially welcomed on this aspect of the interaction. The original author of this patch is Owen Leonard <oleonard@myacpl.org>
Created attachment 14818 [details] [review] [SIGNED-OFF] Bug 4354: Cannot edit existing issuing rules This patch adds javascript which pulls the values from the chosen row of data and populates the last row's form with that data. Some changes have been made to the markup to better accommodate this. Possibly missing is some kind of visual feedback to the user that the edit action is taking place in a different row from where they clicked. Comments are especially welcomed on this aspect of the interaction. The original author of this patch is Owen Leonard <oleonard@myacpl.org> Test Plan: * Edit an existing circulation rule * Delete a circulation rule * Create a new circulation rule * Create a new circulation rule that is a dup of an old one and make sure that it still overwrites the old rule like before Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> All tests passed.
Created attachment 14819 [details] [review] Bug 4354: Cannot edit existing issuing rules This patch adds javascript which pulls the values from the chosen row of data and populates the last row's form with that data. Some changes have been made to the markup to better accommodate this. Possibly missing is some kind of visual feedback to the user that the edit action is taking place in a different row from where they clicked. Comments are especially welcomed on this aspect of the interaction. The original author of this patch is Owen Leonard <oleonard@myacpl.org> Test Plan: * Edit an existing circulation rule * Delete a circulation rule * Create a new circulation rule * Create a new circulation rule that is a dup of an old one and make sure that it still overwrites the old rule like before Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> All tests passed. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 14821 [details] [review] Bug 4354 - Followup - Add row highlighting
(In reply to comment #14) > Created attachment 14821 [details] [review] > Bug 4354 - Followup - Add row highlighting Kyle, I don't see any different in FF on Mac. What am I looking for in the follow up? Nicole
Created attachment 14824 [details] [review] Bug 4354 - Followup - Add row highlighting Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
(In reply to comment #15) > (In reply to comment #14) > > Created attachment 14821 [details] [review] > > Bug 4354 - Followup - Add row highlighting > > Kyle, > > I don't see any different in FF on Mac. What am I looking for in the follow > up? > > Nicole Hi Nicole, It works here with FF, try to reload your page with ctrl-r, clean your cache, etc.
Created attachment 14829 [details] [review] Bug 4354 - Followup - Add row highlighting Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Test plan: * Click on a row to edit * Note that the row is highlighted in orange All tests pass
Before passing qa, this works and adds some usability but it still needs some attention IMO. If I click Edit, I cannot revert my action. There is no Cancel button. If I have clicked Edit and I want to add another rule (old school), it is not possible without clicking save. Could we add a Cancel for that, or have another way out of edit mode? If I edit and enter some values, and click on another Edit, I just lose all my values. I should not have clicked Edit, but could we prevent it? (Not a blocker) I am not a design expert, but cannot resist to say that the orange line is helpful, but it may not be fully clear to every user how this works. (Not a blocker) Please send a followup and switch back to Signed off, or send another awesome response :)
Created attachment 16199 [details] [review] Bug 4354: Cannot edit existing issuing rules This patch adds javascript which pulls the values from the chosen row of data and populates the last row's form with that data. Some changes have been made to the markup to better accommodate this. Possibly missing is some kind of visual feedback to the user that the edit action is taking place in a different row from where they clicked. Comments are especially welcomed on this aspect of the interaction. The original author of this patch is Owen Leonard <oleonard@myacpl.org> Test Plan: * Edit an existing circulation rule * Delete a circulation rule * Create a new circulation rule * Create a new circulation rule that is a dup of an old one and make sure that it still overwrites the old rule like before Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> All tests passed. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 16200 [details] [review] Bug 4354 - Followup - Add row highlighting Test plan: * Click on a row to edit * Note that the row is highlighted in orange Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> All tests pass. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 16234 [details] [review] Bug 4354: Followup Add a Cancel button New cancel button on the edit row. Add a prevent message when editing a new row.
Marcel - taking this bug from you as we are so close to feature freeze. Hope it's ok!
I would really love to see this going in, but I have found a problem with editing the hardduedate setting. When you try to modify a rule that has no hard due date set (shows as: none defined) the field is filled with "defined" on modifying the rule. Also I think this part won't work correctly with translated templates as it seems to rely on the English strings in the template: // specific processing for the Hard due date column var s = itm.split(' '); var select_value; if ( s[0] == "before" ) { select_value = -1 } else if ( s[0] == "on" ) { select_value = 0 } else if ( s[0] == "after" ) { select_value = 1 } Jonathan - could you take a look at that? If it's not too hard to fix switch back to signed off.
Created attachment 16610 [details] [review] Bug 4354: Followup: FIX the hardduedate copy Before this patch, the copy was based on the string (could be translated!). Now is base on the -1, 0, 1 values. The trick is to backup the hardduedatecompare value for each line of the table.
(In reply to comment #24) > Jonathan - could you take a look at that? If it's not too hard to fix switch > back to signed off. Fixed! I switch back to Needs Signoff, I have found a volunteer for testing :)
Patch tested with a sandbox, by Cedric Vita <cedric.vita@dracenie.com>
Created attachment 16611 [details] [review] Bug 4354: Followup: FIX the hardduedate copy Before this patch, the copy was based on the string (could be translated!). Now is base on the -1, 0, 1 values. The trick is to backup the hardduedatecompare value for each line of the table. Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>
Created attachment 16615 [details] [review] Bug 4354: Followup: FIX the hardduedate copy Before this patch, the copy was based on the string (could be translated!). Now is base on the -1, 0, 1 values. The trick is to backup the hardduedatecompare value for each line of the table. Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>
Created attachment 16616 [details] [review] Bug 4354: Followup: FIX the hardduedate copy Before this patch, the copy was based on the string (could be translated!). Now is base on the -1, 0, 1 values. The trick is to backup the hardduedatecompare value for each line of the table. Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>
Created attachment 16617 [details] [review] Bug 4354: Followup: FIX the hardduedate copy Before this patch, the copy was based on the string (could be translated!). Now is base on the -1, 0, 1 values. The trick is to backup the hardduedatecompare value for each line of the table. Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>
Created attachment 16618 [details] [review] Add robots txt file to deny all bots Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>
Comment on attachment 16618 [details] [review] Add robots txt file to deny all bots Wrong file, Paul?
(In reply to comment #38) > Comment on attachment 16618 [details] [review] [review] > Add robots txt file to deny all bots > > Wrong file, Paul? It's not really me in fact, it's a sandbox. I'll ask Cedric Vita, that is the real signoffer, and is in BibLibre office for hackfest ;-)
We are just about there - when you have Current checkouts allowed empty the field should be empty if it says "Unlimited".
Created attachment 16650 [details] [review] Bug 4354: Manage the specific case "Unlimited" value
Created attachment 16654 [details] [review] [PASSED QA] Bug 4354: Cannot edit existing issuing rules This patch adds javascript which pulls the values from the chosen row of data and populates the last row's form with that data. Some changes have been made to the markup to better accommodate this. Possibly missing is some kind of visual feedback to the user that the edit action is taking place in a different row from where they clicked. Comments are especially welcomed on this aspect of the interaction. The original author of this patch is Owen Leonard <oleonard@myacpl.org> Test Plan: * Edit an existing circulation rule * Delete a circulation rule * Create a new circulation rule * Create a new circulation rule that is a dup of an old one and make sure that it still overwrites the old rule like before Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> All tests passed. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Comments on last patch.
Created attachment 16655 [details] [review] [PASSED QA] Bug 4354 - Followup - Add row highlighting Test plan: * Click on a row to edit * Note that the row is highlighted in orange Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> All tests pass. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 16656 [details] [review] [PASSED QA] Bug 4354: Followup Add a Cancel button New cancel button on the edit row. Add a prevent message when editing a new row. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 16657 [details] [review] [PASSED QA] Bug 4354: Followup: FIX the hardduedate copy Before this patch, the copy was based on the string (could be translated!). Now is base on the -1, 0, 1 values. The trick is to backup the hardduedatecompare value for each line of the table. Signed-off-by: Cedric Vita <cedric.vita@dracenie.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 16658 [details] [review] [PASSED QA] Bug 4354: Manage the specific case "Unlimited" value Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All good :)
Katrin, Did you have any comments? The first patch says "Comments on last patch," but the last patch only says "All good," which makes me think I may be missing something somewhere.
This patch has been pushed to master. I made the decision that since all seemed to work in my testing to just push this. If there's a problem I will revert it.
Sorry, I rebased and did my usual pattern - no real comments.