Bug 12365 - Allow descriptive notes to be added to circulation and fine rules
Summary: Allow descriptive notes to be added to circulation and fine rules
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-04 19:38 UTC by Christopher Brannon
Modified: 2020-11-12 15:03 UTC (History)
14 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12365: Adding a note column to circ rules (7.02 KB, patch)
2017-04-05 05:12 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: [FOLLOW-UP] Follow-up patch (1.38 KB, patch)
2017-06-02 03:27 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[MERGE CONFLICT FIX] Bug 12365 - Notes for rules (6.27 KB, patch)
2017-06-19 20:02 UTC, Lee Jamison
Details | Diff | Splinter Review
Bug 12365 - Notes for rules (6.36 KB, patch)
2017-06-19 22:30 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 12365 - Notes for rules (6.42 KB, patch)
2017-06-23 02:58 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 12365 - Notes for rules (6.44 KB, patch)
2017-09-05 03:02 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365 - Notes for rules (6.50 KB, patch)
2017-09-05 03:05 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365 - Notes for rules (6.59 KB, patch)
2017-10-07 13:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12365 - Notes for rules (6.34 KB, patch)
2018-07-02 01:55 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: Notes for rules (6.75 KB, patch)
2018-07-02 15:35 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 12365: [FOLLOW-UP] Adding maxlength attribute and popover (2.97 KB, patch)
2018-07-04 00:34 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: (follow-up) Adding maxlength attribute and popover (5.92 KB, patch)
2018-07-20 02:34 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: (follow-up) Adding maxlength attribute and popover (8.10 KB, patch)
2018-07-26 00:25 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: Notes for rules (7.06 KB, patch)
2018-09-14 18:12 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 12365: (follow-up) Adding maxlength attribute and popover (7.39 KB, patch)
2018-09-14 18:12 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 12365: (follow-up) Putting backticks in atomicupdate (1.12 KB, patch)
2018-10-18 02:44 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: Notes for rules (7.16 KB, patch)
2018-10-19 19:34 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 12365: (follow-up) Adding maxlength attribute and popover (7.48 KB, patch)
2018-10-19 19:35 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 12365: (follow-up) Putting backticks in atomicupdate (1.23 KB, patch)
2018-10-19 19:35 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 12365: (follow-up) Fixing broken circ rules table (1.50 KB, patch)
2018-10-22 23:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: (follow-up) Fixing broken circ rules table (2.33 KB, patch)
2018-10-23 00:07 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: Notes for rules (7.13 KB, patch)
2018-10-30 00:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: (follow-up) Adding maxlength attribute and popover (6.95 KB, patch)
2018-10-30 00:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: (follow-up) Putting backticks in atomicupdate (1.22 KB, patch)
2018-10-30 00:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: (follow-up) Fixing broken circ rules table (2.33 KB, patch)
2018-10-30 00:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 12365: Notes for rules (7.22 KB, patch)
2018-10-31 07:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12365: (follow-up) Adding maxlength attribute and popover (7.01 KB, patch)
2018-10-31 07:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12365: (follow-up) Putting backticks in atomicupdate (1.28 KB, patch)
2018-10-31 07:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12365: (follow-up) Fixing broken circ rules table (2.40 KB, patch)
2018-10-31 07:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12365: Fix GetHardDueDate.t test (832 bytes, patch)
2018-11-08 14:15 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2014-06-04 19:38:16 UTC
I would LOVE to see a notes field for each branch (or default) under smart-rules.pl, if not a note field for each individual rule.

Often we are combing through rules (and settings for that matter), and come across something we are not sure why we set a particular way.  Not all branches have the same workflow or methods, and therefore have a rule set for what they need.  However, when we come across that setting months or years later, we have to go through huge discussions as to why it was set, and if we need that setting.  This would help save a LOT of time, if we could make notes somewhere about it.

Maybe just a notes table, where we could link to the rule, and if there is a note about the rule, a link called "notes" appears next to it that will display a popup window with the note.

Christopher
Comment 1 Christopher Brannon 2014-12-04 19:43:59 UTC
Same would go for preference settings.
Comment 2 Aleisha Amohia 2017-04-05 05:12:40 UTC
Created attachment 61865 [details] [review]
Bug 12365: Adding a note column to circ rules

This patch adds a note column to the circ and fine rules to act as a
description line for the rule.

To test:
1) Apply patch, update database (may have to also update schema)
2) Go to Admin -> Circ and fine rules
3) Scroll to the right, notice new Note column
4) Confirm it works to create a brand new rule with a Note
5) Confirm it works to modify an existing rule with a Note

Sponsored-by: Catalyst IT
Comment 3 Marc Véron 2017-04-06 15:05:47 UTC
Found a minor issue while testing: If you have text in the note field for 'All libraries' + 'All item types', the text 'sticks' in the input field. This is due to setting the value in smart-rules.tt line 384:

<td><input type="text" name="note" id="note" size="15" value="[% rule.note %]"></td>

Works well if I change the line to:
<td><input type="text" name="note" id="note" size="15"></td>
Comment 4 Katrin Fischer 2017-05-02 18:36:17 UTC
61865 - Bug 12365: Adding a note column to circ rules

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 12365: Adding a note column to circ rules
Using index info to reconstruct a base tree...
M	Koha/Schema/Result/Issuingrule.pm
M	admin/smart-rules.pl
M	installer/data/mysql/kohastructure.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging admin/smart-rules.pl
Auto-merging Koha/Schema/Result/Issuingrule.pm
CONFLICT (content): Merge conflict in Koha/Schema/Result/Issuingrule.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 12365: Adding a note column to circ rules
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-12365-Adding-a-note-column-to-circ-rules-pR_pTz.patch
Comment 5 Aleisha Amohia 2017-06-02 03:27:46 UTC Comment hidden (obsolete)
Comment 6 Lee Jamison 2017-06-12 13:49:48 UTC
Unfortunately, still having merge conflicts with the exact conflict message Katrin indicated previously.

Note: I ran reset_all in kohadevbox to guarantee a clean devbox to apply it to.
Comment 7 Lee Jamison 2017-06-19 20:02:02 UTC Comment hidden (obsolete)
Comment 8 Mark Tompsett 2017-06-19 22:30:17 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2017-06-19 22:47:00 UTC
Academy keyword, please do not sign it off. Let it to KohaCon17 hackfesters.
Comment 10 Lee Jamison 2017-06-19 23:19:13 UTC
(In reply to Jonathan Druart from comment #9)
> Academy keyword, please do not sign it off. Let it to KohaCon17 hackfesters.

Thanks for the information...wasn't aware of the significance of the academy keyword!
Comment 11 Biblibre Sandboxes 2017-06-23 02:58:11 UTC
Patch tested with a sandbox, by macon  lauren KohaCon17 <caballeromaricon@gmail.com>
Comment 12 Biblibre Sandboxes 2017-06-23 02:58:35 UTC Comment hidden (obsolete)
Comment 13 Aleisha Amohia 2017-09-05 03:02:58 UTC Comment hidden (obsolete)
Comment 14 Aleisha Amohia 2017-09-05 03:05:01 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2017-10-07 13:36:37 UTC Comment hidden (obsolete)
Comment 16 Josef Moravec 2018-01-02 10:46:06 UTC Comment hidden (obsolete)
Comment 17 Aleisha Amohia 2018-07-02 01:55:30 UTC
Created attachment 76609 [details] [review]
Bug 12365 - Notes for rules

Lee Jamison <ldjamison@marywood.edu> rescued this patch.
It is a clean re-build of attachments 61865 and 63902
to fix the merge conflict introduced during application of both
attachments.

To test:
Follow test plan in Comment 2

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: macon  lauren KohaCon17 <caballeromaricon@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Owen Leonard 2018-07-02 15:35:42 UTC
Created attachment 76637 [details] [review]
Bug 12365: Notes for rules

This patch adds a note column to the circ and fine rules to act as a
description line for the rule.

To test:
1) Apply patch, update database (may have to also update schema)
2) Go to Admin -> Circ and fine rules
3) Scroll to the right, notice new Note column
4) Confirm it works to create a brand new rule with a Note
5) Confirm it works to modify an existing rule with a Note

Sponsored-by: Catalyst IT

Lee Jamison <ldjamison@marywood.edu> rescued this patch.
It is a clean re-build of attachments 61865 and 63902
to fix the merge conflict introduced during application of both
attachments.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: macon  lauren KohaCon17 <caballeromaricon@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 19 Owen Leonard 2018-07-02 15:37:37 UTC
I amended the commit message to include the original test plan.

A couple of follow-ups I'd like to see:

1. Put a maxlength attribute on the form field, since the database column limits to 100 characters.
2. Add some way to show/hide the notes to conserve space, perhaps a popover? https://getbootstrap.com/docs/3.3/javascript/#popovers
Comment 20 Aleisha Amohia 2018-07-04 00:34:27 UTC
Created attachment 76658 [details] [review]
Bug 12365: [FOLLOW-UP] Adding maxlength attribute and popover

Test that you can't add a note of more than 100 characters.
Test notes with rules, notes are hidden by 'Click to view note' and the
popover correctly displays note.
Comment 21 Katrin Fischer 2018-07-08 12:33:40 UTC
Hi Aleisha, thx for the follow-up!

Some notes:

1) I'd like to see "click to view note" changed to just "view note". As the text is formatted as a link, there should be no need to add the additional instruction.

2) When there are multiple lines with notes and you click on one of them, they will just overlay each other, with the first note being behind the second and so on, There seems to be no way to make them disappear again. I think my expectation was that the note would disappear clicking on the next note or just clicking outside the note field again. A longer note can also overlay the buttons, so I think putting some more thought into this would be worthwhile.

3) Tiny: Can you make your patch subject: Bug XXXX: (follow-up)? I would have fixed it as part of QA, but then stumbled on 2). (see: https://wiki.koha-community.org/wiki/Commit_messages#Follow-ups)

Thx!
Comment 22 Katrin Fischer 2018-07-08 12:35:15 UTC
4) Editing a note no longer works!
Comment 23 Aleisha Amohia 2018-07-20 02:34:13 UTC
Created attachment 77147 [details] [review]
Bug 12365: (follow-up) Adding maxlength attribute and popover

Test that you can't add a note of more than 100 characters.
Test notes with rules, notes are hidden by 'View note' and the
popover correctly displays note.
View the note by hovering over the link
I've moved the Note column so the popover displays nicely and doesn't
cover the buttons. I've also changed the placement so that it doesn't
cover information in the row.
Comment 24 Katrin Fischer 2018-07-20 07:43:17 UTC
Aleisha, did you see my comment about not being able to edit the note because it pulls the pop-over note into the input field?
Comment 25 Aleisha Amohia 2018-07-26 00:25:53 UTC
Created attachment 77275 [details] [review]
Bug 12365: (follow-up) Adding maxlength attribute and popover

Test that you can't add a note of more than 100 characters.
Test notes with rules, notes are hidden by 'View note' and the
popover correctly displays note.
View the note by hovering over the link
I've moved the Note column so the popover displays nicely and doesn't
cover the buttons. I've also changed the placement so that it doesn't
cover information in the row.

Update: Test that you can edit the note when clicking 'Edit' and that
behaviour is the same for editing all other fields
Comment 26 Lisette Scheer 2018-09-14 18:02:37 UTC
I got the following error when applying the notes for rules patch. 
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging admin/smart-rules.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 12365: Notes for rules
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-12365-Notes-for-rules-osNzXn.patch

Lisette
Comment 27 Chris Cormack 2018-09-14 18:12:02 UTC
Created attachment 78685 [details] [review]
Bug 12365: Notes for rules

This patch adds a note column to the circ and fine rules to act as a
description line for the rule.

To test:
1) Apply patch, update database (may have to also update schema)
2) Go to Admin -> Circ and fine rules
3) Scroll to the right, notice new Note column
4) Confirm it works to create a brand new rule with a Note
5) Confirm it works to modify an existing rule with a Note

Sponsored-by: Catalyst IT

Lee Jamison <ldjamison@marywood.edu> rescued this patch.
It is a clean re-build of attachments 61865 and 63902
to fix the merge conflict introduced during application of both
attachments.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: macon  lauren KohaCon17 <caballeromaricon@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 28 Chris Cormack 2018-09-14 18:12:28 UTC
Created attachment 78686 [details] [review]
Bug 12365: (follow-up) Adding maxlength attribute and popover

Test that you can't add a note of more than 100 characters.
Test notes with rules, notes are hidden by 'View note' and the
popover correctly displays note.
View the note by hovering over the link
I've moved the Note column so the popover displays nicely and doesn't
cover the buttons. I've also changed the placement so that it doesn't
cover information in the row.

Update: Test that you can edit the note when clicking 'Edit' and that
behaviour is the same for editing all other fields
Comment 29 Lisette Scheer 2018-09-14 18:42:01 UTC
Falling back to patching base and 3-way merge...
error: The following untracked working tree files would be overwritten by merge:
        installer/data/mysql/atomicupdate/bug_12365_-_add_note_column_to_issuingrules.perl
Please move or remove them before you can merge.
Aborting
Failed to merge in the changes.
Patch failed at 0001 Bug 12365: Notes for rules
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-12365-Notes-for-rules-Ei3222.patch

Lisette
Comment 30 Chris Cormack 2018-09-14 18:47:59 UTC
Ahhh that file is left over from the last time you tested, can you checkout master again, then check that file is gone before applying the patch please

Thanks
Chris
Comment 31 Lisette Scheer 2018-09-14 18:59:47 UTC
Was able to apply properly but when trying to add a note both with a new rule and when editing an existing rule, I got the following error: 
No property note for Koha::IssuingRule at /usr/share/perl5/Exception/Class/Base.pm line 73

Lisette
Comment 32 Jonathan Druart 2018-09-14 19:17:42 UTC
Note for QA: This will conflict with bug 15522.
Comment 33 Mark Tompsett 2018-09-14 19:25:13 UTC
Comment on attachment 78685 [details] [review]
Bug 12365: Notes for rules

Review of attachment 78685 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/atomicupdate/bug_12365_-_add_note_column_to_issuingrules.perl
@@ +1,4 @@
> +$DBversion = 'XXX';
> +if( CheckVersion( $DBversion ) ) {
> +    unless( column_exists( 'issuingrules', 'note' ) ) {
> +        $dbh->do(q|ALTER TABLE issuingrules ADD note varchar(100) default NULL AFTER article_requests|);

backticks may be in order.
Comment 34 Katrin Fischer 2018-09-20 21:01:49 UTC
can you explain?
Comment 35 Christopher Brannon 2018-09-24 17:22:59 UTC
I am catching up on bugs, and noticed that it was noted that this is actually modifying the rules table with a new column.  I would like to reiterate, if it isn't too late, that it might be preferable to have a separate notes table.  One that could be used for more than just this one table.  I don't see it hindering performance as a notes table would not be accessed when the rules are used in circulation processes.  If there is a reason it would be better to have the notes in the table, great.  I am learning here, so would be interested in knowing if one method is better than another.
Comment 36 Katrin Fischer 2018-09-24 18:32:42 UTC
Hi Christopher, can you explain what you think the advantage of another table would be? Storing the notes with the data it belongs to makes sense to me on first glance.
Comment 37 Christopher Brannon 2018-09-24 23:25:34 UTC
(In reply to Katrin Fischer from comment #36)
> Hi Christopher, can you explain what you think the advantage of another
> table would be? Storing the notes with the data it belongs to makes sense to
> me on first glance.

The biggest benefit right off the bat is that you wouldn't have to add new columns here and there for the same thing.  If we think we'll need notes for preferences or other settings throughout, we'd end up adding new columns to all of these tables.  Why not create a table that could serve them all?  Create a table once.
Comment 38 Katrin Fischer 2018-09-25 05:26:48 UTC
If you apply the rules of database normalization just a notes column would not need a new table as it's a 1:1 relationship. It would be interesting to see if there are differences in performance or similar, but I think for a single column with little data there might not be much of a difference. I would not block this patch here as it follows an existing pattern.
Comment 39 Jonathan Druart 2018-09-29 15:01:44 UTC
+1 for DB normalization.

(In reply to Christopher Brannon from comment #37)
> The biggest benefit right off the bat is that you wouldn't have to add new
> columns here and there for the same thing.  If we think we'll need notes for
> preferences or other settings throughout, we'd end up adding new columns to
> all of these tables.  Why not create a table that could serve them all? 
> Create a table once.

We will not have foreign keys, it's better to have 1 column per table IMO.
Comment 40 Aleisha Amohia 2018-10-18 02:22:43 UTC
Just wanted to jump in with my input: it makes sense to me that if I'm looking for notes associated to circ rules, I'm going to look at the circ rules table. 

Working on getting this ready for testing.
Comment 41 Aleisha Amohia 2018-10-18 02:44:09 UTC
Created attachment 80790 [details] [review]
Bug 12365: (follow-up) Putting backticks in atomicupdate
Comment 42 Michal Denar 2018-10-19 19:34:54 UTC
Created attachment 80956 [details] [review]
Bug 12365: Notes for rules

This patch adds a note column to the circ and fine rules to act as a
description line for the rule.

To test:
1) Apply patch, update database (may have to also update schema)
2) Go to Admin -> Circ and fine rules
3) Scroll to the right, notice new Note column
4) Confirm it works to create a brand new rule with a Note
5) Confirm it works to modify an existing rule with a Note

Sponsored-by: Catalyst IT

Lee Jamison <ldjamison@marywood.edu> rescued this patch.
It is a clean re-build of attachments 61865 and 63902
to fix the merge conflict introduced during application of both
attachments.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: macon  lauren KohaCon17 <caballeromaricon@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 43 Michal Denar 2018-10-19 19:35:00 UTC
Created attachment 80957 [details] [review]
Bug 12365: (follow-up) Adding maxlength attribute and popover

Test that you can't add a note of more than 100 characters.
Test notes with rules, notes are hidden by 'View note' and the
popover correctly displays note.
View the note by hovering over the link
I've moved the Note column so the popover displays nicely and doesn't
cover the buttons. I've also changed the placement so that it doesn't
cover information in the row.

Update: Test that you can edit the note when clicking 'Edit' and that
behaviour is the same for editing all other fields

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 44 Michal Denar 2018-10-19 19:35:07 UTC
Created attachment 80958 [details] [review]
Bug 12365: (follow-up) Putting backticks in atomicupdate

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 45 Katrin Fischer 2018-10-21 11:11:06 UTC
Hi Aleisha,

please make sure you always run the latest version of the QA tools from the gitlab repository on your patches:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
   FAIL	  filters
		missing_filter at line 125 (                                                                <a name="viewnote" data-toggle="popover" title="Note" data-content="[% rule.note %]" data-placement="top" data-trigger="hover">View note</a>)

I'd have fixed that, but there is another issue: The layout of the issuing rule table is broken with this patch. The buttons at the end of the table are misaligned. 
My entered note shows up in the last column labelled "Actions" as well, pushing the buttons into another non-existing column... (looking at it this will hopefully make more sense)
Comment 46 Aleisha Amohia 2018-10-22 23:43:25 UTC
Created attachment 80997 [details] [review]
Bug 12365: (follow-up) Fixing broken circ rules table
Comment 47 Aleisha Amohia 2018-10-23 00:07:56 UTC
Created attachment 80998 [details] [review]
Bug 12365: (follow-up) Fixing broken circ rules table

Adding filters to note to satisfy qa test tools
Comment 48 Katrin Fischer 2018-10-28 19:01:11 UTC
Follow-up is small enough - setting back to signed off.
Comment 49 Katrin Fischer 2018-10-28 21:55:50 UTC
We got a new column in the table now (daily holds limit) - please rebase on latest master!
Comment 50 Aleisha Amohia 2018-10-30 00:42:48 UTC
Created attachment 81603 [details] [review]
Bug 12365: Notes for rules

This patch adds a note column to the circ and fine rules to act as a
description line for the rule.

To test:
1) Apply patch, update database (may have to also update schema)
2) Go to Admin -> Circ and fine rules
3) Scroll to the right, notice new Note column
4) Confirm it works to create a brand new rule with a Note
5) Confirm it works to modify an existing rule with a Note

Sponsored-by: Catalyst IT

Lee Jamison <ldjamison@marywood.edu> rescued this patch.
It is a clean re-build of attachments 61865 and 63902
to fix the merge conflict introduced during application of both
attachments.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: macon  lauren KohaCon17 <caballeromaricon@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 51 Aleisha Amohia 2018-10-30 00:42:55 UTC
Created attachment 81604 [details] [review]
Bug 12365: (follow-up) Adding maxlength attribute and popover

Test that you can't add a note of more than 100 characters.
Test notes with rules, notes are hidden by 'View note' and the
popover correctly displays note.
View the note by hovering over the link
I've moved the Note column so the popover displays nicely and doesn't
cover the buttons. I've also changed the placement so that it doesn't
cover information in the row.

Update: Test that you can edit the note when clicking 'Edit' and that
behaviour is the same for editing all other fields

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 52 Aleisha Amohia 2018-10-30 00:43:01 UTC
Created attachment 81605 [details] [review]
Bug 12365: (follow-up) Putting backticks in atomicupdate

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 53 Aleisha Amohia 2018-10-30 00:43:08 UTC
Created attachment 81606 [details] [review]
Bug 12365: (follow-up) Fixing broken circ rules table

Adding filters to note to satisfy qa test tools
Comment 54 Aleisha Amohia 2018-10-30 00:43:32 UTC
Rebased. Setting back to signed off
Comment 55 Katrin Fischer 2018-10-31 07:07:32 UTC
Created attachment 81647 [details] [review]
Bug 12365: Notes for rules

This patch adds a note column to the circ and fine rules to act as a
description line for the rule.

To test:
1) Apply patch, update database (may have to also update schema)
2) Go to Admin -> Circ and fine rules
3) Scroll to the right, notice new Note column
4) Confirm it works to create a brand new rule with a Note
5) Confirm it works to modify an existing rule with a Note

Sponsored-by: Catalyst IT

Lee Jamison <ldjamison@marywood.edu> rescued this patch.
It is a clean re-build of attachments 61865 and 63902
to fix the merge conflict introduced during application of both
attachments.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: macon  lauren KohaCon17 <caballeromaricon@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 56 Katrin Fischer 2018-10-31 07:07:38 UTC
Created attachment 81648 [details] [review]
Bug 12365: (follow-up) Adding maxlength attribute and popover

Test that you can't add a note of more than 100 characters.
Test notes with rules, notes are hidden by 'View note' and the
popover correctly displays note.
View the note by hovering over the link
I've moved the Note column so the popover displays nicely and doesn't
cover the buttons. I've also changed the placement so that it doesn't
cover information in the row.

Update: Test that you can edit the note when clicking 'Edit' and that
behaviour is the same for editing all other fields

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 57 Katrin Fischer 2018-10-31 07:07:44 UTC
Created attachment 81649 [details] [review]
Bug 12365: (follow-up) Putting backticks in atomicupdate

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 58 Katrin Fischer 2018-10-31 07:07:49 UTC
Created attachment 81650 [details] [review]
Bug 12365: (follow-up) Fixing broken circ rules table

Adding filters to note to satisfy qa test tools

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 59 Katrin Fischer 2018-10-31 07:08:15 UTC
Thx for your patience and the quick rebase!
Comment 60 Nick Clemens 2018-11-08 13:27:59 UTC
Awesome work all!

Pushed to master for 18.11
Comment 61 Jonathan Druart 2018-11-08 14:15:23 UTC
Created attachment 82081 [details] [review]
Bug 12365: Fix GetHardDueDate.t test
Comment 62 Martin Renvoize 2018-11-16 13:44:55 UTC
Enhancement, will not be backported to 18.05.x series.
Comment 63 Martin Renvoize 2018-11-16 13:46:18 UTC
Can we look to finishing off the removal of the almost exactly the same use case field mentioned in bug 21753 now we have this replacement?