Bug 17268 - Advanced cataloging editor - rancor - macros are lost when browser storage cleared
Summary: Advanced cataloging editor - rancor - macros are lost when browser storage cl...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal with 10 votes (vote)
Assignee: Nick Clemens
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 24789
Blocks: 26929 26954 26955 25494 26964
  Show dependency treegraph
 
Reported: 2016-09-07 15:00 UTC by Nick Clemens
Modified: 2021-12-13 21:08 UTC (History)
15 users (show)

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


Attachments
Bug 17268: Add macros table and permissions (3.96 KB, patch)
2019-10-02 13:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor - rancor - macros are lost when browser storage cleared (23.20 KB, patch)
2019-12-03 15:23 UTC, Cori Lynn Arnold
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.40 KB, patch)
2019-12-23 18:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (42.23 KB, patch)
2019-12-23 18:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH - Schema updates (2.54 KB, patch)
2019-12-23 18:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.43 KB, patch)
2020-01-02 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (42.22 KB, patch)
2020-01-02 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Make shared macros use their own path (35.67 KB, patch)
2020-01-02 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.54 KB, patch)
2020-01-02 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.43 KB, patch)
2020-03-11 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (42.23 KB, patch)
2020-03-11 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Make shared macros use their own path (35.66 KB, patch)
2020-03-11 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Use API to store/retrieve values (15.41 KB, patch)
2020-03-11 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix licenses (3.52 KB, patch)
2020-03-11 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.54 KB, patch)
2020-03-11 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Set Booleaa for shared column in schema (938 bytes, patch)
2020-03-11 13:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.43 KB, patch)
2020-03-11 14:02 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (42.23 KB, patch)
2020-03-11 14:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Make shared macros use their own path (35.66 KB, patch)
2020-03-11 14:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Use API to store/retrieve values (15.32 KB, patch)
2020-03-11 14:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix licenses (3.52 KB, patch)
2020-03-11 14:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.54 KB, patch)
2020-03-11 14:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Set Booleaa for shared column in schema (938 bytes, patch)
2020-03-11 14:03 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: (follow-up) wqUpdate API field definition to boolean (838 bytes, patch)
2020-03-11 16:19 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Handle booleans in JS as such (1.76 KB, patch)
2020-03-11 19:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.43 KB, patch)
2020-03-16 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (52.65 KB, patch)
2020-03-16 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Use API to store/retrieve values (15.47 KB, patch)
2020-03-16 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix licenses (3.52 KB, patch)
2020-03-16 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.54 KB, patch)
2020-03-16 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Set boolean for shared column in schema (938 bytes, patch)
2020-03-16 15:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.43 KB, patch)
2020-03-16 15:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (52.65 KB, patch)
2020-03-16 15:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Use API to store/retrieve values (15.49 KB, patch)
2020-03-16 15:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix licenses (3.52 KB, patch)
2020-03-16 15:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.54 KB, patch)
2020-03-16 15:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Set boolean for shared column in schema (938 bytes, patch)
2020-03-16 15:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.43 KB, patch)
2020-03-16 16:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (52.65 KB, patch)
2020-03-16 16:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Use API to store/retrieve values (15.44 KB, patch)
2020-03-16 16:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix licenses (3.52 KB, patch)
2020-03-16 16:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.54 KB, patch)
2020-03-16 16:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Set boolean for shared column in schema (938 bytes, patch)
2020-03-16 16:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.49 KB, patch)
2020-03-16 17:14 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (52.71 KB, patch)
2020-03-16 17:15 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: Use API to store/retrieve values (15.50 KB, patch)
2020-03-16 17:15 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix licenses (3.58 KB, patch)
2020-03-16 17:15 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.61 KB, patch)
2020-03-16 17:15 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: Set boolean for shared column in schema (1004 bytes, patch)
2020-03-16 17:15 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.55 KB, patch)
2020-03-18 20:43 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (52.77 KB, patch)
2020-03-18 20:43 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: Use API to store/retrieve values (15.56 KB, patch)
2020-03-18 20:43 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix licenses (3.64 KB, patch)
2020-03-18 20:43 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.67 KB, patch)
2020-03-18 20:43 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: Set boolean for shared column in schema (1.04 KB, patch)
2020-03-18 20:43 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix translatability (7.18 KB, patch)
2020-03-27 10:40 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.55 KB, patch)
2020-04-22 21:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (52.77 KB, patch)
2020-04-22 21:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Use API to store/retrieve values (15.56 KB, patch)
2020-04-22 21:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix licenses (3.64 KB, patch)
2020-04-22 21:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.67 KB, patch)
2020-04-22 21:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Set boolean for shared column in schema (1.04 KB, patch)
2020-04-22 21:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix translatability (7.18 KB, patch)
2020-04-22 21:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Update patchs per RFC (22.62 KB, patch)
2020-04-22 21:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 17268: Add macros table and permissions (5.61 KB, patch)
2020-05-01 21:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17268: Advanced cataloging editor macros - add endpoint (52.83 KB, patch)
2020-05-01 21:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17268: Use API to store/retrieve values (15.62 KB, patch)
2020-05-01 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix licenses (3.70 KB, patch)
2020-05-01 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17268: DO NOT PUSH: Schema update (2.72 KB, patch)
2020-05-01 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17268: Set boolean for shared column in schema (1.10 KB, patch)
2020-05-01 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17268: (follow-up) Fix translatability (7.24 KB, patch)
2020-05-01 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17268: Update patchs per RFC (22.68 KB, patch)
2020-05-01 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17268: (QA follow-up) Make "Convert old browser macros" translatable (1.62 KB, patch)
2020-05-01 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17268: (follow-up) Don't show the macro ID in front of description (1.66 KB, patch)
2020-05-01 21:46 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2016-09-07 15:00:34 UTC
In development macros were stored in the browser to allow different users to have their own. Unfortunately this means macros are lost when browser storage is cleared. Should have a more robust method for retaining these.
Comment 1 J Schmidt 2016-09-16 18:17:07 UTC
Additionally, if possible, set up for Koha advanced editor macros should have a built-in shortcut tied to a function key (F1, F2) so that staff may input up to 9 different macros per login. Currently, staff need to mouse click to the field, then mouse click to the macro, then select the macro. The above would make things more efficient. (If you see how CTRL-D is used for inputting a delimiter, it would be like that).
Comment 2 Barbara Johnson 2016-10-20 12:14:18 UTC
I second the need for macros to be tied to function keys since it would greatly improve cataloging efficiency.  In order to increase the number of options to 36 I would add combinations such as CTRL+F1, ALT+F1 and SHIFT+F1.
Comment 3 J Schmidt 2016-11-17 19:40:30 UTC
It sounds as though for Firefox, if you used Options to log into a Firefox account you may be able to sync and preserve cookies from macros from one version of that browser to the next. I'll have to try it.
Comment 4 Heather 2016-11-17 21:59:28 UTC
(In reply to J Schmidt from comment #3)
> It sounds as though for Firefox, if you used Options to log into a Firefox
> account you may be able to sync and preserve cookies from macros from one
> version of that browser to the next. I'll have to try it.

This has been working for me, but I find it's not ideal, since I'd prefer not storing cookies (for more privacy).  I really like the idea of tying macros to function keys--I'd love it even more if I could choose which key combinations to assign to which macros.
Comment 5 Barbara Johnson 2016-11-17 22:15:31 UTC
I agree with being able to customize the key combinations for assigning macros.  What works for me and provides me with an efficient workflow might not work for someone else.
Comment 6 Ron Houk 2019-05-23 15:39:03 UTC
I would also like to see macros store in a more robust way. Or at least have the option to export/import our macros.
Comment 7 kraborn 2019-05-23 18:34:45 UTC
I agree with the previous comments; it would be wonderful for macros to be structured in such a way that they are not wiped out when a browser cache is cleared. The option of tying the macros to various function keys would be a fabulous enhancement and make the macros an even more powerful tool. The functionality of exporting and importing macros would also be very helpful for training new catalogers and supporting consistency across a team.
Comment 8 Nick Clemens 2019-10-02 13:31:38 UTC
Created attachment 93474 [details] [review]
Bug 17268: Add macros table and permissions
Comment 9 Nick Clemens 2019-10-02 13:33:22 UTC
Working on moving the macros into the DB

I think we should add a REST route for CRUD and new permissions for public macros

Please file a new bug for mapping macros to hotkeys :-)
Comment 10 Cori Lynn Arnold 2019-12-03 15:23:34 UTC
Created attachment 95948 [details] [review]
Bug 17268: Advanced cataloging editor - rancor - macros are lost when browser storage cleared

First pass at api code
Comment 11 Nick Clemens 2019-12-23 18:52:34 UTC
Created attachment 96616 [details] [review]
Bug 17268: Add macros table and permissions
Comment 12 Nick Clemens 2019-12-23 18:52:38 UTC
Created attachment 96617 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint
Comment 13 Nick Clemens 2019-12-23 18:52:41 UTC
Created attachment 96618 [details] [review]
Bug 17268: DO NOT PUSH - Schema updates
Comment 14 Nick Clemens 2020-01-02 15:16:25 UTC
Created attachment 96757 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared
Comment 15 Nick Clemens 2020-01-02 15:16:28 UTC
Created attachment 96758 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint
Comment 16 Nick Clemens 2020-01-02 15:16:31 UTC
Created attachment 96759 [details] [review]
Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public
Comment 17 Nick Clemens 2020-01-02 15:16:34 UTC
Created attachment 96760 [details] [review]
Bug 17268: DO NOT PUSH: Schema update
Comment 18 Katrin Fischer 2020-01-02 21:54:34 UTC
Not ready for testing yet?
Comment 19 Nick Clemens 2020-03-11 13:41:20 UTC
Created attachment 100559 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared
Comment 20 Nick Clemens 2020-03-11 13:41:23 UTC
Created attachment 100560 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint
Comment 21 Nick Clemens 2020-03-11 13:41:27 UTC
Created attachment 100561 [details] [review]
Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public
Comment 22 Nick Clemens 2020-03-11 13:41:30 UTC
Created attachment 100562 [details] [review]
Bug 17268: Use API to store/retrieve values

To test:
 1  - Enable AdvancedCatalogingEditor
 2  - Add some macros (they don't need to be valid, just have content)
 3  - Apply patches
 4  - updatedatabase
 5  - Set user to have 'create_shared_macros' and 'delete_shared_macros'
 6  - Load the advanced editor (Cataloging->Advanced editor)
 7  - Click on 'Macros'
 8  - Previous macros should not show, but you should have a 'Convert' button
 9  - Convert old macros and confirm they show
10  - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11  - Have at least on valid macro and run it, e.g.:
    new 100=Testing this out
12  - Run the macro, confirm it runs
13  - Try a macro with gibberish, confirm there is an error when running
14  - Ensure you have a few macros marked public
15  - In a private browser window sign in as a patron with neither shared macro permission
16  - Confirm the public macros load, but cannot be edited
17  - Grant create_shared_macros permission to this patron
18  - Reload editor, they should now be able to edit shared macros
19  - Confirm they cannot delete shared macros
20  - Grant delete_shared_macros permission
21  - Reload editor
22  - Confirm they can now delete shared macros
Comment 23 Nick Clemens 2020-03-11 13:41:33 UTC
Created attachment 100563 [details] [review]
Bug 17268: (follow-up) Fix licenses
Comment 24 Nick Clemens 2020-03-11 13:41:37 UTC
Created attachment 100564 [details] [review]
Bug 17268: DO NOT PUSH: Schema update
Comment 25 Nick Clemens 2020-03-11 13:41:40 UTC
Created attachment 100565 [details] [review]
Bug 17268: Set Booleaa for shared column in schema
Comment 26 Nick Clemens 2020-03-11 14:02:57 UTC
Created attachment 100569 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared
Comment 27 Nick Clemens 2020-03-11 14:03:01 UTC
Created attachment 100570 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint
Comment 28 Nick Clemens 2020-03-11 14:03:05 UTC
Created attachment 100571 [details] [review]
Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public
Comment 29 Nick Clemens 2020-03-11 14:03:08 UTC
Created attachment 100572 [details] [review]
Bug 17268: Use API to store/retrieve values

To test:
 1  - Enable AdvancedCatalogingEditor
 2  - Add some macros (they don't need to be valid, just have content)
 3  - Apply patches
 4  - updatedatabase
 5  - Set user to have 'create_shared_macros' and 'delete_shared_macros'
 6  - Load the advanced editor (Cataloging->Advanced editor)
 7  - Click on 'Macros'
 8  - Previous macros should not show, but you should have a 'Convert' button
 9  - Convert old macros and confirm they show
10  - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11  - Have at least on valid macro and run it, e.g.:
    new 100=Testing this out
12  - Run the macro, confirm it runs
13  - Try a macro with gibberish, confirm there is an error when running
14  - Ensure you have a few macros marked public
15  - In a private browser window sign in as a patron with neither shared macro permission
16  - Confirm the public macros load, but cannot be edited
17  - Grant create_shared_macros permission to this patron
18  - Reload editor, they should now be able to edit shared macros
19  - Confirm they cannot delete shared macros
20  - Grant delete_shared_macros permission
21  - Reload editor
22  - Confirm they can now delete shared macros
Comment 30 Nick Clemens 2020-03-11 14:03:11 UTC
Created attachment 100573 [details] [review]
Bug 17268: (follow-up) Fix licenses
Comment 31 Nick Clemens 2020-03-11 14:03:15 UTC
Created attachment 100574 [details] [review]
Bug 17268: DO NOT PUSH: Schema update
Comment 32 Nick Clemens 2020-03-11 14:03:18 UTC
Created attachment 100575 [details] [review]
Bug 17268: Set Booleaa for shared column in schema
Comment 33 Andrew Fuerste-Henry 2020-03-11 16:07:46 UTC
I'm failing out around step 7. I've applied patches and updated database. When I go to adv editor as either a superlibrarian or a user with all cataloging permissions, when I click Macros I don't see a Convert button. Also, when I click New Macro, give it a name, and click OK I get this error: 

Failed to create macro:{"errors":[{"message":"anyOf failed: Expected string or null, got boolean.","path":"\/shared"}],"status":500}

Jumping over to the database, I see the macro name in advanced_editor_macros, but it doesn't list on the macros page.

If leave adv editor, come back in, click Macros again I get the error:

Failed to load macros:{"errors":[{"message":"anyOf failed: Expected string or null, got boolean.","path":"\/0\/shared"}],"status":500}
Comment 34 Nick Clemens 2020-03-11 16:19:13 UTC
Created attachment 100584 [details] [review]
Bug 17268: (follow-up) wqUpdate API field definition to boolean
Comment 35 Andrew Fuerste-Henry 2020-03-11 16:28:03 UTC
Am now getting a slightly different error in the same place:

Failed to create macro:{"errors":[{"message":"anyOf failed: Expected boolean or null, got number.","path":"\/body\/shared"}],"status":400}
Comment 36 Tomás Cohen Arazi 2020-03-11 19:09:45 UTC
Created attachment 100604 [details] [review]
Bug 17268: Handle booleans in JS as such

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 37 Tomás Cohen Arazi 2020-03-11 19:10:49 UTC
(In reply to Tomás Cohen Arazi from comment #36)
> Created attachment 100604 [details] [review] [review]
> Bug 17268: Handle booleans in JS as such
> 
> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Andrew, I submitted a follow-up so you can properly test the feature.
Nick, squash it if you feel like :-D
Comment 38 Andrew Fuerste-Henry 2020-03-16 13:27:59 UTC
Macros I had in my cache before applying patches are not visible after patch, so I cannot convert them. Failing QA for this.

I also got a weird error that I cannot seem to recreate:
- create user with only "catalogue" and "editcatalogue" permissions
- open incognito window, log in as your user
- create a new macro, mark it public
- click away from your new macro
- click back to your new macro to edit it, get this error:
"Failed to load macros:{"error":"This macro is shared, you must access it via advancededitormacros\/shared"}"

But it lets you edit the macro anyway? And it still shows and works for other users?
And then once I'd logged out and back in again, the error stopped. As I said, I haven't been able to make it happen again.
Comment 39 Nick Clemens 2020-03-16 15:16:40 UTC
Created attachment 100758 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared
Comment 40 Nick Clemens 2020-03-16 15:16:44 UTC
Created attachment 100759 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint

Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public

Bug 17268: (follow-up) Update API field definition to boolean
Comment 41 Nick Clemens 2020-03-16 15:16:48 UTC
Created attachment 100760 [details] [review]
Bug 17268: Use API to store/retrieve values

To test:
 1  - Enable AdvancedCatalogingEditor
 2  - Add some macros (they don't need to be valid, just have content)
 3  - Apply patches
 4  - updatedatabase
 5  - Set user to have 'create_shared_macros' and 'delete_shared_macros'
 6  - Load the advanced editor (Cataloging->Advanced editor)
 7  - Click on 'Macros'
 8  - Previous macros should not show, but you should have a 'Convert' button
 9  - Convert old macros and confirm they show
10  - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11  - Have at least on valid macro and run it, e.g.:
    new 100=Testing this out
12  - Run the macro, confirm it runs
13  - Try a macro with gibberish, confirm there is an error when running
14  - Ensure you have a few macros marked public
15  - In a private browser window sign in as a patron with neither shared macro permission
16  - Confirm the public macros load, but cannot be edited
17  - Grant create_shared_macros permission to this patron
18  - Reload editor, they should now be able to edit shared macros
19  - Confirm they cannot delete shared macros
20  - Grant delete_shared_macros permission
21  - Reload editor
22  - Confirm they can now delete shared macros
Comment 42 Nick Clemens 2020-03-16 15:16:51 UTC
Created attachment 100761 [details] [review]
Bug 17268: (follow-up) Fix licenses
Comment 43 Nick Clemens 2020-03-16 15:16:55 UTC
Created attachment 100762 [details] [review]
Bug 17268: DO NOT PUSH: Schema update
Comment 44 Nick Clemens 2020-03-16 15:16:59 UTC
Created attachment 100763 [details] [review]
Bug 17268: Set boolean for shared column in schema
Comment 45 Nick Clemens 2020-03-16 15:57:40 UTC
Created attachment 100766 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared
Comment 46 Nick Clemens 2020-03-16 15:57:44 UTC
Created attachment 100767 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint

Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public

Bug 17268: (follow-up) Update API field definition to boolean
Comment 47 Nick Clemens 2020-03-16 15:57:48 UTC
Created attachment 100768 [details] [review]
Bug 17268: Use API to store/retrieve values

To test:
 1  - Enable AdvancedCatalogingEditor
 2  - Add some macros (they don't need to be valid, just have content)
 3  - Apply patches
 4  - updatedatabase
 5  - Set user to have 'create_shared_macros' and 'delete_shared_macros'
 6  - Load the advanced editor (Cataloging->Advanced editor)
 7  - Click on 'Macros'
 8  - Previous macros should not show, but you should have a 'Convert' button
 9  - Convert old macros and confirm they show
10  - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11  - Have at least on valid macro and run it, e.g.:
    new 100=Testing this out
12  - Run the macro, confirm it runs
13  - Try a macro with gibberish, confirm there is an error when running
14  - Ensure you have a few macros marked public
15  - In a private browser window sign in as a patron with neither shared macro permission
16  - Confirm the public macros load, but cannot be edited
17  - Grant create_shared_macros permission to this patron
18  - Reload editor, they should now be able to edit shared macros
19  - Confirm they cannot delete shared macros
20  - Grant delete_shared_macros permission
21  - Reload editor
22  - Confirm they can now delete shared macros
Comment 48 Nick Clemens 2020-03-16 15:57:52 UTC
Created attachment 100769 [details] [review]
Bug 17268: (follow-up) Fix licenses
Comment 49 Nick Clemens 2020-03-16 15:57:55 UTC
Created attachment 100770 [details] [review]
Bug 17268: DO NOT PUSH: Schema update
Comment 50 Nick Clemens 2020-03-16 15:57:59 UTC
Created attachment 100771 [details] [review]
Bug 17268: Set boolean for shared column in schema
Comment 51 Nick Clemens 2020-03-16 16:05:34 UTC
Created attachment 100772 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared
Comment 52 Nick Clemens 2020-03-16 16:05:38 UTC
Created attachment 100773 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint

Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public

Bug 17268: (follow-up) Update API field definition to boolean
Comment 53 Nick Clemens 2020-03-16 16:05:42 UTC
Created attachment 100774 [details] [review]
Bug 17268: Use API to store/retrieve values

To test:
 1  - Enable AdvancedCatalogingEditor
 2  - Add some macros (they don't need to be valid, just have content)
 3  - Apply patches
 4  - updatedatabase
 5  - Set user to have 'create_shared_macros' and 'delete_shared_macros'
 6  - Load the advanced editor (Cataloging->Advanced editor)
 7  - Click on 'Macros'
 8  - Previous macros should not show, but you should have a 'Convert' button
 9  - Convert old macros and confirm they show
10  - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11  - Have at least on valid macro and run it, e.g.:
    new 100=Testing this out
12  - Run the macro, confirm it runs
13  - Try a macro with gibberish, confirm there is an error when running
14  - Ensure you have a few macros marked public
15  - In a private browser window sign in as a patron with neither shared macro permission
16  - Confirm the public macros load, but cannot be edited
17  - Grant create_shared_macros permission to this patron
18  - Reload editor, they should now be able to edit shared macros
19  - Confirm they cannot delete shared macros
20  - Grant delete_shared_macros permission
21  - Reload editor
22  - Confirm they can now delete shared macros
Comment 54 Nick Clemens 2020-03-16 16:05:46 UTC
Created attachment 100775 [details] [review]
Bug 17268: (follow-up) Fix licenses
Comment 55 Nick Clemens 2020-03-16 16:05:50 UTC
Created attachment 100776 [details] [review]
Bug 17268: DO NOT PUSH: Schema update
Comment 56 Nick Clemens 2020-03-16 16:05:53 UTC
Created attachment 100777 [details] [review]
Bug 17268: Set boolean for shared column in schema
Comment 57 Andrew Fuerste-Henry 2020-03-16 17:14:58 UTC
Created attachment 100780 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 58 Andrew Fuerste-Henry 2020-03-16 17:15:04 UTC
Created attachment 100781 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint

Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public

Bug 17268: (follow-up) Update API field definition to boolean

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 59 Andrew Fuerste-Henry 2020-03-16 17:15:08 UTC
Created attachment 100782 [details] [review]
Bug 17268: Use API to store/retrieve values

To test:
 1  - Enable AdvancedCatalogingEditor
 2  - Add some macros (they don't need to be valid, just have content)
 3  - Apply patches
 4  - updatedatabase
 5  - Set user to have 'create_shared_macros' and 'delete_shared_macros'
 6  - Load the advanced editor (Cataloging->Advanced editor)
 7  - Click on 'Macros'
 8  - Previous macros should not show, but you should have a 'Convert' button
 9  - Convert old macros and confirm they show
10  - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11  - Have at least on valid macro and run it, e.g.:
    new 100=Testing this out
12  - Run the macro, confirm it runs
13  - Try a macro with gibberish, confirm there is an error when running
14  - Ensure you have a few macros marked public
15  - In a private browser window sign in as a patron with neither shared macro permission
16  - Confirm the public macros load, but cannot be edited
17  - Grant create_shared_macros permission to this patron
18  - Reload editor, they should now be able to edit shared macros
19  - Confirm they cannot delete shared macros
20  - Grant delete_shared_macros permission
21  - Reload editor
22  - Confirm they can now delete shared macros

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 60 Andrew Fuerste-Henry 2020-03-16 17:15:12 UTC
Created attachment 100783 [details] [review]
Bug 17268: (follow-up) Fix licenses

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 61 Andrew Fuerste-Henry 2020-03-16 17:15:15 UTC
Created attachment 100784 [details] [review]
Bug 17268: DO NOT PUSH: Schema update

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 62 Andrew Fuerste-Henry 2020-03-16 17:15:19 UTC
Created attachment 100785 [details] [review]
Bug 17268: Set boolean for shared column in schema

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 63 Andrew Fuerste-Henry 2020-03-18 20:43:10 UTC
Created attachment 100983 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 64 Andrew Fuerste-Henry 2020-03-18 20:43:16 UTC
Created attachment 100984 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint

Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public

Bug 17268: (follow-up) Update API field definition to boolean

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 65 Andrew Fuerste-Henry 2020-03-18 20:43:19 UTC
Created attachment 100985 [details] [review]
Bug 17268: Use API to store/retrieve values

To test:
 1  - Enable AdvancedCatalogingEditor
 2  - Add some macros (they don't need to be valid, just have content)
 3  - Apply patches
 4  - updatedatabase
 5  - Set user to have 'create_shared_macros' and 'delete_shared_macros'
 6  - Load the advanced editor (Cataloging->Advanced editor)
 7  - Click on 'Macros'
 8  - Previous macros should not show, but you should have a 'Convert' button
 9  - Convert old macros and confirm they show
10  - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11  - Have at least on valid macro and run it, e.g.:
    new 100=Testing this out
12  - Run the macro, confirm it runs
13  - Try a macro with gibberish, confirm there is an error when running
14  - Ensure you have a few macros marked public
15  - In a private browser window sign in as a patron with neither shared macro permission
16  - Confirm the public macros load, but cannot be edited
17  - Grant create_shared_macros permission to this patron
18  - Reload editor, they should now be able to edit shared macros
19  - Confirm they cannot delete shared macros
20  - Grant delete_shared_macros permission
21  - Reload editor
22  - Confirm they can now delete shared macros

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 66 Andrew Fuerste-Henry 2020-03-18 20:43:23 UTC
Created attachment 100986 [details] [review]
Bug 17268: (follow-up) Fix licenses

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 67 Andrew Fuerste-Henry 2020-03-18 20:43:27 UTC
Created attachment 100987 [details] [review]
Bug 17268: DO NOT PUSH: Schema update

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 68 Andrew Fuerste-Henry 2020-03-18 20:43:31 UTC
Created attachment 100988 [details] [review]
Bug 17268: Set boolean for shared column in schema

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 69 Heather 2020-03-18 20:51:45 UTC
I'm so happy to have had a chance to help this along--this would save me SO MUCH time in cataloging and would allow me to create macros for others cataloging at my institution to use so would not only save them time but would enhance the quality and accuracy of our cataloging records immensely.  Anything that anyone can do to help this along would be most appreciated!!
Comment 70 Jonathan Druart 2020-03-24 09:38:52 UTC
Nick, do we have precedents for adding stuffs REST API side only?
Here you are adding permission checks in Koha/REST/V1, not in the regular Koha object class.
Comment 71 Jonathan Druart 2020-03-24 09:40:26 UTC
On another note I think the error messages are not translatable (but I did not test, that may be wrong).
Comment 72 Katrin Fischer 2020-03-26 21:45:33 UTC
Having a look here too. Database update and permissions look good. Seems to work well, although I have some concerns about macros working correctly in our current version (see 5)

Found only some small stuff that needs fixing:

1) QA test tool fail
 FAIL	Koha/AdvancedEditorMacros.pm
   FAIL	  pod coverage
		POD is missing for 'object_class'
   OK	  spelling

2) Translatability

Jonathan, I think you are right, if these show in the GUI (which it looks like) they won't be translatable.

openapi => { error => "To create shared macros you must use advancededitor/shared" } );
openapi => { error => "To create private macros you must use advancededitor" } );

There are also some other glitches in the Javascript code:
+            $convert = $( '<button class="btn btn-default" id="convert-macros" title="Convert browser storage macros"><i class="fa fa-adjust"></i> Convert old macros</button>' );

3) Should these API additions be voted on?

4) Capitalization

I've noticed lots of glitches there in patches recently - makes my inner librarian unhappy:

+      "description": "Advanced Editor Macro internal identifier",

5) Testing went well, but I am wondering if there is an issue with our MACRO language. A lot of the examples I tried from the wiki would not work:

https://wiki.koha-community.org/wiki/Advanced_editor_macros

Especially the RDA macros that add multiple subfields - only the first subfield code will be recognized. I assume they used to work (trusting Heather), so what happened there?

As this is so heavy on API it would be nice to get Tomas to have a look too.
Comment 73 Andrew Fuerste-Henry 2020-03-26 22:04:28 UTC
I'm sad to say, those RDA macros work in master without these patches. Just tested this one on master and it made all three new fields:
new 336=‡atext ǂb txt ǂ2 rdacontent
new 337=‡aunmediated ǂb n ǂ2 rdamedia
new 338=‡avolume ǂb nc ǂ2 rdacarrier

If it's only making one of the three with these patches then that ain't good.
Comment 74 Tomás Cohen Arazi 2020-03-26 22:14:40 UTC
The fact it relies on the API should be a good thing for everyone, no one wants to add new CGI controller scripts that can only be tested with selenium :-D

About translatability, the JS libraries usually just detect faulty codes (4XX, 5XX, and so on). And the error message is not what gets presented to the end user. That should be handled as usual in JS with translatability in mind. For example, if we do

GET /a/resource/12

and get a 404, it doesn't matter the JSON that is returned (even though we try to be consistent on the format and texts there). The code on the UI should handle the fact that the requested resource doesn't exist, based on the 404 code.

That said, maybe Nick relied on those messages for the UI, and that should be fixed. I haven't checked that myself.

Regarding the need for a vote, I'd say we should, yes. This patchset requires something different than others: the resources (macros) can belong to a patron, but can also be shared. And we can only specify AND conditions for resource access. So we cannot say (on the spec) things like: 'you can edit this if you are superlibrarian or the owner'. It could be worked out in a different way, but I don't think we can avoid (at least) a minimum check in the controller, and is just fine IMHO.

I will recheck this patchset in deep and provide more feedback ASAP. Specially if some design change can make this cleaner to implement. I haven't found a better option, thus adding Martin and Agustin to the conversation to gather more ideas.
Comment 75 Katrin Fischer 2020-03-26 22:49:41 UTC
(In reply to Andrew Fuerste-Henry from comment #73)
> I'm sad to say, those RDA macros work in master without these patches. Just
> tested this one on master and it made all three new fields:
> new 336=‡atext ǂb txt ǂ2 rdacontent
> new 337=‡aunmediated ǂb n ǂ2 rdamedia
> new 338=‡avolume ǂb nc ǂ2 rdacarrier
> 
> If it's only making one of the three with these patches then that ain't good.

Do the subfield codes show up the right 'color' for you? I have the text added by the macro, but only the first is a real 'subfield' delimeter somehow. When I save and look at the record, they show in the record as ǂb and ǂ2 in normal view.
Comment 76 Jonathan Druart 2020-03-27 07:33:53 UTC
(In reply to Tomás Cohen Arazi from comment #74)
> The fact it relies on the API should be a good thing for everyone, no one
> wants to add new CGI controller scripts that can only be tested with
> selenium :-D

The problem is having all the logic in Koha::REST, how do you do if you want to reuse this code outside of a REST API call?
Comment 77 Nick Clemens 2020-03-27 10:40:14 UTC
Created attachment 101980 [details] [review]
Bug 17268: (follow-up) Fix translatability

This patch moves the error messages to translatable strings

Also includes capitalization fixes and POD adjustment
Comment 78 Nick Clemens 2020-03-27 10:56:27 UTC
(In reply to Katrin Fischer from comment #72)
> 1) QA test tool fail

Fixed - new POD complaint is false positive

> 2) Translatability

Fixed
 
> There are also some other glitches in the Javascript code:

Fixed

> 3) Should these API additions be voted on?

https://wiki.koha-community.org/wiki/Advanced_editor_macros_endpoint_RFC

> 4) Capitalization

Fixed

> 5) Testing went well, but I am wondering if there is an issue with our MACRO
> language. A lot of the examples I tried from the wiki would not work:

Several of the wiki examples had bad characters
ǂ != ‡ - fixing those made the macros work for me
Comment 79 Nick Clemens 2020-03-27 11:08:58 UTC
(In reply to Jonathan Druart from comment #76)
> (In reply to Tomás Cohen Arazi from comment #74)
> > The fact it relies on the API should be a good thing for everyone, no one
> > wants to add new CGI controller scripts that can only be tested with
> > selenium :-D
> 
> The problem is having all the logic in Koha::REST, how do you do if you want
> to reuse this code outside of a REST API call?

The logic in the API is mostly permission based, and pretty light in my opinion. Why are you interacting with this code outside of the API, and what are you trying to do?
Comment 80 Tomás Cohen Arazi 2020-03-27 11:12:04 UTC
(In reply to Jonathan Druart from comment #76)
> (In reply to Tomás Cohen Arazi from comment #74)
> > The fact it relies on the API should be a good thing for everyone, no one
> > wants to add new CGI controller scripts that can only be tested with
> > selenium :-D
> 
> The problem is having all the logic in Koha::REST, how do you do if you want
> to reuse this code outside of a REST API call?


Well, we try not to do that. In this case there's really no business logic besides permission checking, which we traditionally do in the controllers.
Comment 81 Katrin Fischer 2020-03-27 13:08:25 UTC
> Several of the wiki examples had bad characters
> ǂ != ‡ - fixing those made the macros work for me

That's a relief! Thx!
Comment 82 Katrin Fischer 2020-03-27 13:11:28 UTC
Leaving this for the moment in Tomas' and Nick's hands to resolve and hope to see things move soon!
Comment 83 Agustín Moyano 2020-03-27 13:30:36 UTC
(In reply to Tomás Cohen Arazi from comment #80)
> (In reply to Jonathan Druart from comment #76)
> > (In reply to Tomás Cohen Arazi from comment #74)
> > > The fact it relies on the API should be a good thing for everyone, no one
> > > wants to add new CGI controller scripts that can only be tested with
> > > selenium :-D
> > 
> > The problem is having all the logic in Koha::REST, how do you do if you want
> > to reuse this code outside of a REST API call?
> 
> 
> Well, we try not to do that. In this case there's really no business logic
> besides permission checking, which we traditionally do in the controllers.

First of all, really great work Nick!

* About the endpoint path, if we are talking about patron's macros, I believe it would make more sense to have something like

GET /api/v1/patrons/{patron_id}/advancededitormacros
GET /api/v1/patrons/{patron_id}/advancededitormacros/{advancededitormacro_id}
etc...

Meaning patron_id should be in the path

* An idea for having OR conditions for resources access is to have it specified in x-koha-authorization, for example

"x-koha-authorization": {
    "permissions": {
        "editcatalogue": "advanced_editor"
    },
    "or-condition": {
        "shared": 1
    }
}

and have object.search build that dbic query whit the patron_id it founds in the path.. permissions will always be checked before this in Auth.pm.

Regards
Comment 84 Katrin Fischer 2020-04-12 22:45:20 UTC
I think we just voted on a little change with the RFC - not sure if more work is need, Nick, can you comment please?

https://wiki.koha-community.org/wiki/Advanced_editor_macros_endpoint_RFC

Note: 
Seems to be missng from the summary page (https://wiki.koha-community.org/wiki/REST_api_RFCs)
Comment 85 Nick Clemens 2020-04-22 21:15:27 UTC
Created attachment 103507 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 86 Nick Clemens 2020-04-22 21:15:31 UTC
Created attachment 103508 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint

Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public

Bug 17268: (follow-up) Update API field definition to boolean

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 87 Nick Clemens 2020-04-22 21:15:35 UTC
Created attachment 103509 [details] [review]
Bug 17268: Use API to store/retrieve values

To test:
 1  - Enable AdvancedCatalogingEditor
 2  - Add some macros (they don't need to be valid, just have content)
 3  - Apply patches
 4  - updatedatabase
 5  - Set user to have 'create_shared_macros' and 'delete_shared_macros'
 6  - Load the advanced editor (Cataloging->Advanced editor)
 7  - Click on 'Macros'
 8  - Previous macros should not show, but you should have a 'Convert' button
 9  - Convert old macros and confirm they show
10  - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11  - Have at least on valid macro and run it, e.g.:
    new 100=Testing this out
12  - Run the macro, confirm it runs
13  - Try a macro with gibberish, confirm there is an error when running
14  - Ensure you have a few macros marked public
15  - In a private browser window sign in as a patron with neither shared macro permission
16  - Confirm the public macros load, but cannot be edited
17  - Grant create_shared_macros permission to this patron
18  - Reload editor, they should now be able to edit shared macros
19  - Confirm they cannot delete shared macros
20  - Grant delete_shared_macros permission
21  - Reload editor
22  - Confirm they can now delete shared macros

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 88 Nick Clemens 2020-04-22 21:15:40 UTC
Created attachment 103510 [details] [review]
Bug 17268: (follow-up) Fix licenses

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 89 Nick Clemens 2020-04-22 21:15:43 UTC
Created attachment 103511 [details] [review]
Bug 17268: DO NOT PUSH: Schema update

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 90 Nick Clemens 2020-04-22 21:15:48 UTC
Created attachment 103512 [details] [review]
Bug 17268: Set boolean for shared column in schema

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Comment 91 Nick Clemens 2020-04-22 21:15:53 UTC
Created attachment 103513 [details] [review]
Bug 17268: (follow-up) Fix translatability

This patch moves the error messages to translatable strings

Also includes capitalization fixes and POD adjustment
Comment 93 Katrin Fischer 2020-05-01 21:45:49 UTC
Created attachment 104144 [details] [review]
Bug 17268: Add macros table and permissions

Bug 17268: Change public to shared

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 94 Katrin Fischer 2020-05-01 21:45:55 UTC
Created attachment 104145 [details] [review]
Bug 17268: Advanced cataloging editor macros - add endpoint

Bug 17268: Make shared macros use their own path

Bug 17268: Update API to use shared instead of public

Bug 17268: (follow-up) Update API field definition to boolean

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 95 Katrin Fischer 2020-05-01 21:46:01 UTC
Created attachment 104146 [details] [review]
Bug 17268: Use API to store/retrieve values

To test:
 1  - Enable AdvancedCatalogingEditor
 2  - Add some macros (they don't need to be valid, just have content)
 3  - Apply patches
 4  - updatedatabase
 5  - Set user to have 'create_shared_macros' and 'delete_shared_macros'
 6  - Load the advanced editor (Cataloging->Advanced editor)
 7  - Click on 'Macros'
 8  - Previous macros should not show, but you should have a 'Convert' button
 9  - Convert old macros and confirm they show
10  - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11  - Have at least on valid macro and run it, e.g.:
    new 100=Testing this out
12  - Run the macro, confirm it runs
13  - Try a macro with gibberish, confirm there is an error when running
14  - Ensure you have a few macros marked public
15  - In a private browser window sign in as a patron with neither shared macro permission
16  - Confirm the public macros load, but cannot be edited
17  - Grant create_shared_macros permission to this patron
18  - Reload editor, they should now be able to edit shared macros
19  - Confirm they cannot delete shared macros
20  - Grant delete_shared_macros permission
21  - Reload editor
22  - Confirm they can now delete shared macros

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 96 Katrin Fischer 2020-05-01 21:46:07 UTC
Created attachment 104147 [details] [review]
Bug 17268: (follow-up) Fix licenses

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 97 Katrin Fischer 2020-05-01 21:46:12 UTC
Created attachment 104148 [details] [review]
Bug 17268: DO NOT PUSH: Schema update

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 98 Katrin Fischer 2020-05-01 21:46:17 UTC
Created attachment 104149 [details] [review]
Bug 17268: Set boolean for shared column in schema

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 99 Katrin Fischer 2020-05-01 21:46:23 UTC
Created attachment 104150 [details] [review]
Bug 17268: (follow-up) Fix translatability

This patch moves the error messages to translatable strings

Also includes capitalization fixes and POD adjustment

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 100 Katrin Fischer 2020-05-01 21:46:28 UTC
Created attachment 104151 [details] [review]
Bug 17268: Update patchs per RFC

https://wiki.koha-community.org/wiki/Advanced_editor_macros_endpoint_RFC

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 101 Katrin Fischer 2020-05-01 21:46:34 UTC
Created attachment 104152 [details] [review]
Bug 17268: (QA follow-up) Make "Convert old browser macros" translatable

The code was missing the necessary markup to make the string translatable.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 102 Katrin Fischer 2020-05-01 21:46:40 UTC
Created attachment 104153 [details] [review]
Bug 17268: (follow-up) Don't show the macro ID in front of description

I believe it's nicer to allow users to organize the macros as
they want using the descrpitions they prefer.

Submitting as a suggestion.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 103 Katrin Fischer 2020-05-01 21:47:09 UTC
Last patch is a suggestion - but easiest to show in the code, what I mean :)
Comment 104 Nick Clemens 2020-05-01 21:56:13 UTC
(In reply to Katrin Fischer from comment #103)
> Last patch is a suggestion - but easiest to show in the code, what I mean :)

I am happy with that. I think I only had id because you can reuse the same name, but this works. Thank you! So happy to see this :-)
Comment 105 Martin Renvoize 2020-05-04 07:41:53 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 106 Joy Nelson 2020-05-08 23:13:41 UTC
not backported to 19.11