Bug 35919 - Add record sources CRUD
Summary: Add record sources CRUD
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Jonathan Druart
URL:
Keywords:
: 32607 (view as bug list)
Depends on: 33169 32925 33066 34215
Blocks: 35956 37419 31791
  Show dependency treegraph
 
Reported: 2024-01-26 13:49 UTC by Tomás Cohen Arazi
Modified: 2024-07-22 08:57 UTC (History)
15 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/887
Text to go in the release notes:
This development creates a new entity in Koha, the `record sources`. They will be used to define policies for records based on their source.
Version(s) released in:
24.05.00


Attachments
Bug 35919: DB changes (10.52 KB, patch)
2024-01-26 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add Koha::RecordSource(s) classes (2.90 KB, patch)
2024-01-26 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add /record_sources endpoints (23.88 KB, patch)
2024-01-26 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add record sources admin page (22.34 KB, patch)
2024-01-26 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: DBIC schema [DO NOT PUSH] (3.64 KB, patch)
2024-01-26 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add Koha::Biblio::Metadata->can_be_edited (2.33 KB, patch)
2024-01-26 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add cypress tests (7.42 KB, patch)
2024-01-26 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: DB changes (10.59 KB, patch)
2024-01-29 10:35 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35919: Add Koha::RecordSource(s) classes (2.98 KB, patch)
2024-01-29 10:35 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35919: Add /record_sources endpoints (23.95 KB, patch)
2024-01-29 10:35 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35919: Add record sources admin page (22.40 KB, patch)
2024-01-29 10:35 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35919: DBIC schema [DO NOT PUSH] (3.71 KB, patch)
2024-01-29 10:35 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35919: Add Koha::Biblio::Metadata->can_be_edited (2.45 KB, patch)
2024-01-29 10:35 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35919: Add cypress tests (7.50 KB, patch)
2024-01-29 10:35 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35919: Add superlibrarian permission to Permissions.t (1.29 KB, patch)
2024-02-27 09:32 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35919: Fix label for 'can be edited' checkbox (1.09 KB, patch)
2024-02-29 13:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Fix edit route to match existing ones (1.73 KB, patch)
2024-02-29 13:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Adjust RewriteRule to match existing ones (1.14 KB, patch)
2024-02-29 13:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Adjust RewriteRule and routes to match existing ones (4.26 KB, patch)
2024-02-29 13:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: DB changes (10.55 KB, patch)
2024-03-18 16:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add Koha::RecordSource(s) classes (2.97 KB, patch)
2024-03-18 16:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add /record_sources endpoints (23.94 KB, patch)
2024-03-18 16:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add record sources admin page (23.72 KB, patch)
2024-03-18 16:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: DBIC schema [DO NOT PUSH] (3.70 KB, patch)
2024-03-18 16:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add Koha::Biblio::Metadata->can_be_edited (2.43 KB, patch)
2024-03-18 16:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add cypress tests (7.45 KB, patch)
2024-03-18 16:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add superlibrarian permission to Permissions.t (1.28 KB, patch)
2024-03-18 16:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add superlibrarian permission to Permissions.t (1.33 KB, patch)
2024-03-18 16:39 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Add Koha::Biblio::Metadata->source_allows_editing (2.73 KB, patch)
2024-03-18 18:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35919: Adjust h1 (1.20 KB, patch)
2024-03-19 11:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Fix cypress tests (3.01 KB, patch)
2024-03-19 11:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: DB changes (10.63 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Add Koha::RecordSource(s) classes (3.04 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Add /record_sources endpoints (24.01 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Add record sources admin page (23.79 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: DBIC schema [DO NOT PUSH] (3.77 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Add cypress tests (7.52 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Add superlibrarian permission to Permissions.t (1.41 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Add Koha::Biblio::Metadata->source_allows_editing (2.80 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Adjust h1 (1.27 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: Fix cypress tests (3.08 KB, patch)
2024-03-19 11:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35919: DB changes (10.63 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: Add Koha::RecordSource(s) classes (3.04 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: Add /record_sources endpoints (24.01 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: Add record sources admin page (23.44 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: DBIC schema [DO NOT PUSH] (3.77 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: Add cypress tests (7.52 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: Add superlibrarian permission to Permissions.t (1.41 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: Add Koha::Biblio::Metadata->source_allows_editing (2.80 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: Adjust h1 (1.27 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: Fix cypress tests (3.08 KB, patch)
2024-04-11 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35919: (QA follow-up) Address QA concerns (4.88 KB, patch)
2024-04-22 12:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2024-01-26 13:49:07 UTC
In order to implement bug 31791, a way to defined record sources is needed.

This bug replaces bug 32607 (it will be closed) in order to make it easier to track things.
Comment 1 Tomás Cohen Arazi 2024-01-26 13:49:27 UTC
*** Bug 32607 has been marked as a duplicate of this bug. ***
Comment 2 Tomás Cohen Arazi 2024-01-26 13:58:00 UTC
Created attachment 161531 [details] [review]
Bug 35919: DB changes

This patch adds:

* A new table: `record_sources`.
* A new user permission: `manage_record_sources`.
* A FK on record_sources.recour_source_id on the biblio_metadata tables

Record sources will contain a name and (for now) a flag telling if
records from the specific source can be manually edited.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2024-01-26 13:58:03 UTC
Created attachment 161532 [details] [review]
Bug 35919: Add Koha::RecordSource(s) classes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi 2024-01-26 13:58:05 UTC
Created attachment 161533 [details] [review]
Bug 35919: Add /record_sources endpoints

This patch introduces endpoints for managing record sources. This is
done on top of Koha::RecordSource(s) following the current coding style.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/record_sources.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Tomás Cohen Arazi 2024-01-26 13:58:08 UTC
Created attachment 161534 [details] [review]
Bug 35919: Add record sources admin page

This patch introduces a Vue.js based record sources managing page. To
test it:

1. Apply this patch
2. Build the Vue.js stuff:
   $ ktd --shell
  k$ yarn js:build
  k$ restart_all
3. On the staff interface, go to Administration > Record sources
4. Play with the interface and the offered actions
=> SUCCESS: Things go well
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2024-01-26 13:58:11 UTC
Created attachment 161535 [details] [review]
Bug 35919: DBIC schema [DO NOT PUSH]

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi 2024-01-26 13:58:13 UTC
Created attachment 161537 [details] [review]
Bug 35919: Add Koha::Biblio::Metadata->can_be_edited

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Tomás Cohen Arazi 2024-01-26 13:58:16 UTC
Created attachment 161538 [details] [review]
Bug 35919: Add cypress tests

This patch adds Cypress tests for the `record sources` CRUD as required
by QA.

I tried to cover all the UI interactions and behaviours.

To test:
1. Have all the patches applied
2. Run:
   $ ktd --shell
  k$ cypress run --spec t/cypress/integration/Admin/RecordSources_spec.ts
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Matt Blenkinsop 2024-01-29 10:35:32 UTC
Created attachment 161577 [details] [review]
Bug 35919: DB changes

This patch adds:

* A new table: `record_sources`.
* A new user permission: `manage_record_sources`.
* A FK on record_sources.recour_source_id on the biblio_metadata tables

Record sources will contain a name and (for now) a flag telling if
records from the specific source can be manually edited.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 10 Matt Blenkinsop 2024-01-29 10:35:35 UTC
Created attachment 161578 [details] [review]
Bug 35919: Add Koha::RecordSource(s) classes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 11 Matt Blenkinsop 2024-01-29 10:35:38 UTC
Created attachment 161579 [details] [review]
Bug 35919: Add /record_sources endpoints

This patch introduces endpoints for managing record sources. This is
done on top of Koha::RecordSource(s) following the current coding style.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/record_sources.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 12 Matt Blenkinsop 2024-01-29 10:35:40 UTC
Created attachment 161580 [details] [review]
Bug 35919: Add record sources admin page

This patch introduces a Vue.js based record sources managing page. To
test it:

1. Apply this patch
2. Build the Vue.js stuff:
   $ ktd --shell
  k$ yarn js:build
  k$ restart_all
3. On the staff interface, go to Administration > Record sources
4. Play with the interface and the offered actions
=> SUCCESS: Things go well
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 13 Matt Blenkinsop 2024-01-29 10:35:43 UTC
Created attachment 161581 [details] [review]
Bug 35919: DBIC schema [DO NOT PUSH]

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 14 Matt Blenkinsop 2024-01-29 10:35:46 UTC
Created attachment 161582 [details] [review]
Bug 35919: Add Koha::Biblio::Metadata->can_be_edited

prove t/db_dependent/Koha/Biblio/Metadata.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 15 Matt Blenkinsop 2024-01-29 10:35:49 UTC
Created attachment 161583 [details] [review]
Bug 35919: Add cypress tests

This patch adds Cypress tests for the `record sources` CRUD as required
by QA.

I tried to cover all the UI interactions and behaviours.

To test:
1. Have all the patches applied
2. Run:
   $ ktd --shell
  k$ cypress run --spec t/cypress/integration/Admin/RecordSources_spec.ts
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 16 Matt Blenkinsop 2024-01-29 10:36:47 UTC
The UI works as advertised and the unit tests pass. I also found the unit test for t/db_dependent/Koha/Biblio/Metadata.t which I've added to one of the commit messages
Comment 17 Jonathan Druart 2024-01-30 08:09:00 UTC
(In reply to Tomás Cohen Arazi from comment #0)
> This bug replaces bug 32607 (it will be closed) in order to make it easier
> to track things.

Easier by hiding the history?...
Comment 18 Tomás Cohen Arazi 2024-02-13 15:08:08 UTC
(In reply to Jonathan Druart from comment #17)
> (In reply to Tomás Cohen Arazi from comment #0)
> > This bug replaces bug 32607 (it will be closed) in order to make it easier
> > to track things.
> 
> Easier by hiding the history?...

- Some generalizations got dropped.
- Some generalizations were taken care of on separate bugs already in master.
- Some features were simplified to make this easier to match the client's MVP, and future enhancements will be dealt with on follow-up bugs in which pertinent discussions can take place in a better way.

I think that's the best way to do it.
Comment 19 Matt Blenkinsop 2024-02-27 09:32:37 UTC
Created attachment 162489 [details] [review]
Bug 35919: Add superlibrarian permission to Permissions.t

Permissions.t was failing when running a jenkins build to another branch with this bug applied. Permissions.t was missing the permission for record sources and is now added in this patch

Test plan:
1) prove t/Koha/Auth/Permissions.t
Comment 20 Jonathan Druart 2024-02-29 13:00:04 UTC
Created attachment 162594 [details] [review]
Bug 35919: Fix label for 'can be edited' checkbox
Comment 21 Jonathan Druart 2024-02-29 13:00:07 UTC
Created attachment 162595 [details] [review]
Bug 35919: Fix edit route to match existing ones
Comment 22 Jonathan Druart 2024-02-29 13:01:26 UTC
Created attachment 162596 [details] [review]
Bug 35919: Adjust RewriteRule to match existing ones
Comment 23 Jonathan Druart 2024-02-29 13:24:35 UTC
Created attachment 162599 [details] [review]
Bug 35919: Adjust RewriteRule and routes to match existing ones
Comment 24 Jonathan Druart 2024-02-29 13:36:45 UTC
I don't understand why you stubborn and obstinate to do things differently. I have said that 10x on bug 32607, then here again.

1. computed title instead of the logic in template
2. retrieving the client api from setup()
3. _has_value should not be needed, we have form validation already
4. you should clean the api-client, why do you deal with headers and json stringify? we don't need it in other places!
5. in the routes:
    BeforeUnloadEvent() {
        window.location.href = "/cgi-bin/koha/admin/admin-home.pl";
    },
Why?
6. Why do you name the add/edit component "Edit" when all others are named "AddForm"?

7. Why calling the client api this way:
  .then(() => something())
when all existing occurrences do:
  .then(
    success => something(),
    error => {},
  );
8.Edit.vue
    Object.keys(response).forEach(key => {
        this.row[key] = response[key]
    })
What's the point of this??

Some are blocker, some are weirdness, others are better than what exist but different.

I let you decide the status of this.
Comment 25 Jonathan Druart 2024-02-29 15:31:44 UTC
Also wondering if the routes should not have a more specific names: "Edit", "List", could clash by other modules.
Comment 26 Tomás Cohen Arazi 2024-03-08 18:15:39 UTC
(In reply to Jonathan Druart from comment #24)
> I don't understand why you stubborn and obstinate to do things differently.
> I have said that 10x on bug 32607, then here again.

Maybe because some things didn't exist when this was submitted in the first place? Or maybe some things were rewritten by the time this got (finally) some eyes on it.

I really thought I had covered all the remaining things. I removed all of the contentious snippets.

I will try to understand your concerns. Vue is not my whellhouse. This is the first time I work with Vue, trying to help someone else's dev to move forward. And even learnt Cypress and wrote the darn tests for a trivial CRUD page!

> 1. computed title instead of the logic in template
> 2. retrieving the client api from setup()
> 3. _has_value should not be needed, we have form validation already
> 4. you should clean the api-client, why do you deal with headers and json
> stringify? we don't need it in other places!
> 5. in the routes:
>     BeforeUnloadEvent() {
>         window.location.href = "/cgi-bin/koha/admin/admin-home.pl";
>     },
> Why?
> 6. Why do you name the add/edit component "Edit" when all others are named
> "AddForm"?
> 
> 7. Why calling the client api this way:
>   .then(() => something())
> when all existing occurrences do:
>   .then(
>     success => something(),
>     error => {},
>   );
> 8.Edit.vue
>     Object.keys(response).forEach(key => {
>         this.row[key] = response[key]
>     })
> What's the point of this??
> 
> Some are blocker, some are weirdness, others are better than what exist but
> different.
> 
> I let you decide the status of this.
Comment 27 Jonathan Druart 2024-03-11 12:39:32 UTC
Just ask if you have questions or need help!
Comment 28 Marcel de Rooy 2024-03-15 07:17:11 UTC
If no one beats me to it, will be looking here "soon"
Comment 29 Tomás Cohen Arazi 2024-03-15 13:04:23 UTC
(In reply to Marcel de Rooy from comment #28)
> If no one beats me to it, will be looking here "soon"

I will try to address Jonathan's QA concerns.
Comment 30 Tomás Cohen Arazi 2024-03-18 16:28:08 UTC
Created attachment 163348 [details] [review]
Bug 35919: DB changes

This patch adds:

* A new table: `record_sources`.
* A new user permission: `manage_record_sources`.
* A FK on record_sources.recour_source_id on the biblio_metadata tables

Record sources will contain a name and (for now) a flag telling if
records from the specific source can be manually edited.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 31 Tomás Cohen Arazi 2024-03-18 16:28:11 UTC
Created attachment 163349 [details] [review]
Bug 35919: Add Koha::RecordSource(s) classes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 32 Tomás Cohen Arazi 2024-03-18 16:28:15 UTC
Created attachment 163350 [details] [review]
Bug 35919: Add /record_sources endpoints

This patch introduces endpoints for managing record sources. This is
done on top of Koha::RecordSource(s) following the current coding style.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/record_sources.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 33 Tomás Cohen Arazi 2024-03-18 16:28:18 UTC
Created attachment 163351 [details] [review]
Bug 35919: Add record sources admin page

This patch introduces a Vue.js based record sources managing page. To
test it:

1. Apply this patch
2. Build the Vue.js stuff:
   $ ktd --shell
  k$ yarn js:build
  k$ restart_all
3. On the staff interface, go to Administration > Record sources
4. Play with the interface and the offered actions
=> SUCCESS: Things go well
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 34 Tomás Cohen Arazi 2024-03-18 16:28:21 UTC
Created attachment 163352 [details] [review]
Bug 35919: DBIC schema [DO NOT PUSH]

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 35 Tomás Cohen Arazi 2024-03-18 16:28:25 UTC
Created attachment 163353 [details] [review]
Bug 35919: Add Koha::Biblio::Metadata->can_be_edited

prove t/db_dependent/Koha/Biblio/Metadata.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 36 Tomás Cohen Arazi 2024-03-18 16:28:28 UTC
Created attachment 163354 [details] [review]
Bug 35919: Add cypress tests

This patch adds Cypress tests for the `record sources` CRUD as required
by QA.

I tried to cover all the UI interactions and behaviours.

To test:
1. Have all the patches applied
2. Run:
   $ ktd --shell
  k$ cypress run --spec t/cypress/integration/Admin/RecordSources_spec.ts
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 37 Tomás Cohen Arazi 2024-03-18 16:28:31 UTC Comment hidden (obsolete)
Comment 38 Tomás Cohen Arazi 2024-03-18 16:39:42 UTC
Created attachment 163360 [details] [review]
Bug 35919: Add superlibrarian permission to Permissions.t

Permissions.t was failing when running a jenkins build to another branch with this bug applied. Permissions.t was missing the permission for record sources and is now added in this patch

Test plan:
1) prove t/Koha/Auth/Permissions.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 39 Tomás Cohen Arazi 2024-03-18 16:52:01 UTC
Marking NSO again, as I rewrote most of the Vue part of this dev.

Some things I'm not comfortable with, but happy to follow the current standard practice for now, and evolve later.

I kept the URL rewrite rule we originally submitted. I think requiring the trailing slash (/) if my rewrite avoids it, doesn't make sense.

I can submit a bug for redoing the rest of the rewrites this way if required by QA (if this applies, as I'm not familiar with the little details the other modules might have making them requiring this).

I also liked it more when the API client was instantiated globally instead on each place it is used. As it is a stateless object it should cause no issues. I moved it to match the current practice and will file a separate bug to restore the original implementation and discuss it in place.

Thanks everyone!
Comment 40 Tomás Cohen Arazi 2024-03-18 18:14:46 UTC
Created attachment 163369 [details] [review]
Bug 35919: Add Koha::Biblio::Metadata->source_allows_editing

This patch adds a convenient method that tells if the record is
editable, according to it's record source.

To test:
1. Apply this patch
2. Run:
   $ ktd
  k$ prove t/db_dependent/Koha/Biblio/Metadata.t
=> SUCCESS: Tests pass! Use cases covered!
3. Sign off :-D

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Bug 31791: Rename can_be_edited -> source_allows_editing
Comment 41 Jonathan Druart 2024-03-19 11:15:14 UTC
1. 
Create a record source, edit it
Modify the name
h1 is getting modified. I don't think it's expected.
Comment 42 Jonathan Druart 2024-03-19 11:18:02 UTC
Created attachment 163389 [details] [review]
Bug 35919: Adjust h1
Comment 43 Jonathan Druart 2024-03-19 11:24:49 UTC
Created attachment 163390 [details] [review]
Bug 35919: Fix cypress tests

And remove "remove", "delete" inconsistency
Comment 44 Jonathan Druart 2024-03-19 11:28:16 UTC
Created attachment 163391 [details] [review]
Bug 35919: DB changes

This patch adds:

* A new table: `record_sources`.
* A new user permission: `manage_record_sources`.
* A FK on record_sources.recour_source_id on the biblio_metadata tables

Record sources will contain a name and (for now) a flag telling if
records from the specific source can be manually edited.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 45 Jonathan Druart 2024-03-19 11:28:19 UTC
Created attachment 163392 [details] [review]
Bug 35919: Add Koha::RecordSource(s) classes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 46 Jonathan Druart 2024-03-19 11:28:23 UTC
Created attachment 163393 [details] [review]
Bug 35919: Add /record_sources endpoints

This patch introduces endpoints for managing record sources. This is
done on top of Koha::RecordSource(s) following the current coding style.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/record_sources.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 47 Jonathan Druart 2024-03-19 11:28:27 UTC
Created attachment 163394 [details] [review]
Bug 35919: Add record sources admin page

This patch introduces a Vue.js based record sources managing page. To
test it:

1. Apply this patch
2. Build the Vue.js stuff:
   $ ktd --shell
  k$ yarn js:build
  k$ restart_all
3. On the staff interface, go to Administration > Record sources
4. Play with the interface and the offered actions
=> SUCCESS: Things go well
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 48 Jonathan Druart 2024-03-19 11:28:30 UTC
Created attachment 163395 [details] [review]
Bug 35919: DBIC schema [DO NOT PUSH]

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 49 Jonathan Druart 2024-03-19 11:28:33 UTC
Created attachment 163396 [details] [review]
Bug 35919: Add cypress tests

This patch adds Cypress tests for the `record sources` CRUD as required
by QA.

I tried to cover all the UI interactions and behaviours.

To test:
1. Have all the patches applied
2. Run:
   $ ktd --shell
  k$ cypress run --spec t/cypress/integration/Admin/RecordSources_spec.ts
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 50 Jonathan Druart 2024-03-19 11:28:37 UTC
Created attachment 163397 [details] [review]
Bug 35919: Add superlibrarian permission to Permissions.t

Permissions.t was failing when running a jenkins build to another branch with this bug applied. Permissions.t was missing the permission for record sources and is now added in this patch

Test plan:
1) prove t/Koha/Auth/Permissions.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 51 Jonathan Druart 2024-03-19 11:28:40 UTC
Created attachment 163398 [details] [review]
Bug 35919: Add Koha::Biblio::Metadata->source_allows_editing

This patch adds a convenient method that tells if the record is
editable, according to it's record source.

To test:
1. Apply this patch
2. Run:
   $ ktd
  k$ prove t/db_dependent/Koha/Biblio/Metadata.t
=> SUCCESS: Tests pass! Use cases covered!
3. Sign off :-D

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Bug 31791: Rename can_be_edited -> source_allows_editing

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 52 Jonathan Druart 2024-03-19 11:28:43 UTC
Created attachment 163399 [details] [review]
Bug 35919: Adjust h1

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 53 Jonathan Druart 2024-03-19 11:28:46 UTC
Created attachment 163400 [details] [review]
Bug 35919: Fix cypress tests

And remove "remove", "delete" inconsistency

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 54 Tomás Cohen Arazi 2024-03-19 14:44:12 UTC
Comment on attachment 163399 [details] [review]
Bug 35919: Adjust h1

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

::: koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/RecordSources/FormAdd.vue
@@ +5,5 @@
> +            {{
> +                $__("Edit record source #%s").format(
> +                    record_source.record_source_id
> +                )
> +            }}

I get the change from the name to the ID for consistency. But I would like to know how you end up with that coding style so I do it right next time.

I used whatever the editor suggested, and then used the prettify tool blindly so far.
Comment 55 Martin Renvoize 2024-03-19 14:56:22 UTC
If it's not already, we should document how to setup editors for these prettier styling requirements.
Comment 56 Jonathan Druart 2024-03-19 14:57:18 UTC
I am just using git commit and the pre-commit hook is doing its thing (ie. calling prettier).

What should be done for you, as you should have the hook setup as well.
Comment 57 Jonathan Druart 2024-03-19 14:58:01 UTC
It's actually splitting because the line is longer I guess.
Comment 58 Tomás Cohen Arazi 2024-03-19 14:59:51 UTC
(In reply to Jonathan Druart from comment #57)
> It's actually splitting because the line is longer I guess.

+1 that's it
Comment 59 Jonathan Druart 2024-03-20 07:55:43 UTC
(In reply to Tomás Cohen Arazi from comment #39)
> Marking NSO again, as I rewrote most of the Vue part of this dev.
> 
> Some things I'm not comfortable with, but happy to follow the current
> standard practice for now, and evolve later.
> 
> I kept the URL rewrite rule we originally submitted. I think requiring the
> trailing slash (/) if my rewrite avoids it, doesn't make sense.
> 
> I can submit a bug for redoing the rest of the rewrites this way if required
> by QA (if this applies, as I'm not familiar with the little details the
> other modules might have making them requiring this).
> 
> I also liked it more when the API client was instantiated globally instead
> on each place it is used. As it is a stateless object it should cause no
> issues. I moved it to match the current practice and will file a separate
> bug to restore the original implementation and discuss it in place.
> 
> Thanks everyone!

Did you open the bugs?
Comment 60 Martin Renvoize 2024-04-11 13:46:14 UTC
Created attachment 164708 [details] [review]
Bug 35919: DB changes

This patch adds:

* A new table: `record_sources`.
* A new user permission: `manage_record_sources`.
* A FK on record_sources.recour_source_id on the biblio_metadata tables

Record sources will contain a name and (for now) a flag telling if
records from the specific source can be manually edited.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 61 Martin Renvoize 2024-04-11 13:46:17 UTC
Created attachment 164709 [details] [review]
Bug 35919: Add Koha::RecordSource(s) classes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 62 Martin Renvoize 2024-04-11 13:46:21 UTC
Created attachment 164710 [details] [review]
Bug 35919: Add /record_sources endpoints

This patch introduces endpoints for managing record sources. This is
done on top of Koha::RecordSource(s) following the current coding style.

To test:
1. Apply this patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/record_sources.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 63 Martin Renvoize 2024-04-11 13:46:25 UTC
Created attachment 164711 [details] [review]
Bug 35919: Add record sources admin page

This patch introduces a Vue.js based record sources managing page. To
test it:

1. Apply this patch
2. Build the Vue.js stuff:
   $ ktd --shell
  k$ yarn js:build
  k$ restart_all
3. On the staff interface, go to Administration > Record sources
4. Play with the interface and the offered actions
=> SUCCESS: Things go well
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 64 Martin Renvoize 2024-04-11 13:46:30 UTC
Created attachment 164712 [details] [review]
Bug 35919: DBIC schema [DO NOT PUSH]

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 65 Martin Renvoize 2024-04-11 13:46:33 UTC
Created attachment 164713 [details] [review]
Bug 35919: Add cypress tests

This patch adds Cypress tests for the `record sources` CRUD as required
by QA.

I tried to cover all the UI interactions and behaviours.

To test:
1. Have all the patches applied
2. Run:
   $ ktd --shell
  k$ cypress run --spec t/cypress/integration/Admin/RecordSources_spec.ts
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 66 Martin Renvoize 2024-04-11 13:46:37 UTC
Created attachment 164714 [details] [review]
Bug 35919: Add superlibrarian permission to Permissions.t

Permissions.t was failing when running a jenkins build to another branch with this bug applied. Permissions.t was missing the permission for record sources and is now added in this patch

Test plan:
1) prove t/Koha/Auth/Permissions.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 67 Martin Renvoize 2024-04-11 13:46:41 UTC
Created attachment 164715 [details] [review]
Bug 35919: Add Koha::Biblio::Metadata->source_allows_editing

This patch adds a convenient method that tells if the record is
editable, according to it's record source.

To test:
1. Apply this patch
2. Run:
   $ ktd
  k$ prove t/db_dependent/Koha/Biblio/Metadata.t
=> SUCCESS: Tests pass! Use cases covered!
3. Sign off :-D

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>

Bug 31791: Rename can_be_edited -> source_allows_editing

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 68 Martin Renvoize 2024-04-11 13:46:44 UTC
Created attachment 164716 [details] [review]
Bug 35919: Adjust h1

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 69 Martin Renvoize 2024-04-11 13:46:49 UTC
Created attachment 164717 [details] [review]
Bug 35919: Fix cypress tests

And remove "remove", "delete" inconsistency

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 70 Katrin Fischer 2024-04-22 06:31:44 UTC
I am sorry, some very small issues, but also a bigger one I couldn't get around (5):

1) Page title

We since changed the markup there for better translatability among other things. Please adapt:

+<title>
+    Record sources &rsaquo; Koha
+</title>

2) Missing permissions.inc

Guess everyone has only been testing as a superlibrarian again? ;)

Wanted to fix in a follow-up, but got stuck on 5) :(

3) Reconsider sequence on admin page

Currently this shows as the last entry below the Elasticsearch config, but I feel it would be better suited with the matching/overlay rules, what do you think?

4) Missing from side navigation

The left hand admin navigation panel was missed. Sequence should match admin page, so good to do 3) first :)

5) Apache config?

When trying to access the admin/record_sources I have a 404 error.

I tried yarn build, restart_all and reset_all.

I figure this line here might not be right?
RewriteRule ^/cgi-bin/koha/admin/record_sources(.*)?$ /cgi-bin/koha/admin/record_sources.pl$1 [PT]

I verified it showed in /etc/koha/apache-shared-intranet.conf
Comment 71 Tomás Cohen Arazi 2024-04-22 12:28:18 UTC
Created attachment 165274 [details] [review]
Bug 35919: (QA follow-up) Address QA concerns

This patch:

* Adds the missing entry in permissions.inc
* Moves the configuration entry above 'Record overlay rules'
* Adds the 'Record sources' link to the left navigation column, in the
  right position.
* Makes the <title> element be constructed the same way the 'cities'
  page is.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 72 Tomás Cohen Arazi 2024-04-22 12:32:36 UTC
(In reply to Katrin Fischer from comment #70)
> 1) Page title
> 
> We since changed the markup there for better translatability among other
> things. Please adapt:
> 
> +<title>
> +    Record sources &rsaquo; Koha
> +</title>

Fixed. It would be great if it was documented somewhere or there was something in the guidelines about this. I copied what's done in the other Vue apps as requested by QA, and they don't implement this currently. Done anyway.

> 2) Missing permissions.inc
> 
> Guess everyone has only been testing as a superlibrarian again? ;)
> 
> Wanted to fix in a follow-up, but got stuck on 5) :(

Fixed.

> 3) Reconsider sequence on admin page
> 
> Currently this shows as the last entry below the Elasticsearch config, but I
> feel it would be better suited with the matching/overlay rules, what do you
> think?

I agree. Done.

> 4) Missing from side navigation
> 
> The left hand admin navigation panel was missed. Sequence should match admin
> page, so good to do 3) first :)

Right, good catch. Done. Preserving the order discussed above.

> 5) Apache config?
> 
> When trying to access the admin/record_sources I have a 404 error.
> 
> I tried yarn build, restart_all and reset_all.

I'm not sure what's going on on your KTD, but I was playing with 'main' and just:

```shell
git bz apply 35919
ktd --shell
reset-all
```

and the record sources page is there.
Comment 73 Kyle M Hall 2024-04-22 17:05:28 UTC
> 5) Apache config?
> 
> When trying to access the admin/record_sources I have a 404 error.

I applied the patch, did a restart_all, and it looks to be working for me!
Comment 74 Katrin Fischer 2024-04-26 15:06:49 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 75 Fridolin Somers 2024-05-24 07:51:00 UTC
Not backported to 23.11.x
Comment 76 Caroline Cyr La Rose 2024-07-04 21:05:31 UTC
I did my best to understand and describe this new feature (along with bug 31791). Please see the merge request and let me know if I what I wrote is wrong! https://gitlab.com/koha-community/koha-manual/-/merge_requests/887