Bug 27266 - Move C4::Biblio::GetMarcAuthors to Koha namespace
Summary: Move C4::Biblio::GetMarcAuthors to Koha namespace
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 3150
  Show dependency treegraph
 
Reported: 2020-12-17 23:07 UTC by Aleisha Amohia
Modified: 2021-07-22 20:30 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 27266: Move GetMarcAuthors to Koha namespace (17.91 KB, patch)
2020-12-18 01:11 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 27266: Move GetMarcAuthors to Koha namespace (17.73 KB, patch)
2020-12-21 21:38 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 27266: Move GetMarcAuthors to Koha namespace (17.78 KB, patch)
2021-02-24 06:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 27266: Move GetMarcAuthors to Koha namespace (17.30 KB, patch)
2021-03-16 11:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27266: Move GetMarcAuthors to Koha namespace (17.44 KB, patch)
2021-03-16 11:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27266: Move GetMarcAuthors to Koha namespace (17.76 KB, patch)
2021-07-21 21:25 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 27266: (QA follow-up) Rename to get_marc_authors (6.39 KB, patch)
2021-07-21 21:25 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 27266: (follow-up) Remove instances of GetMarcAuthors (3.70 KB, patch)
2021-07-21 22:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 27266: Move GetMarcAuthors to Koha namespace (17.74 KB, patch)
2021-07-22 07:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27266: (QA follow-up) Rename to get_marc_authors (6.46 KB, patch)
2021-07-22 07:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27266: (follow-up) Remove instances of GetMarcAuthors (3.76 KB, patch)
2021-07-22 07:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27266: (follow-up) Using biblio frameworkcode to get rid of FIXME (1017 bytes, patch)
2021-07-22 20:30 UTC, Aleisha Amohia
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2020-12-17 23:07:58 UTC

    
Comment 1 Aleisha Amohia 2020-12-18 01:11:49 UTC Comment hidden (obsolete)
Comment 2 Aleisha Amohia 2020-12-21 21:38:54 UTC
Created attachment 114580 [details] [review]
Bug 27266: Move GetMarcAuthors to Koha namespace

This patch moves C4::Biblio::GetMarcAuthors to
Koha::Biblio->get_authors_from_MARC. This is so the method can be
used in templates and notices.

To test:
1. Find a record that has an author in the added entry field (700-720).
2. Add the record to the cart and a list.
3. View your cart and click 'more details'. Confirm authors show as
normal.
4. Click 'send' and confirm the email sent shows the authors as normal.
5. Go to the list you added the record to and click 'send list'. Confirm
the email sent shows the authors as normal.
6. Set the OPACXSLTDetailsDisplay system preference to empty (for no
xslt)
7. Log in to the OPAC. Find the record and add it to the cart and a list
8. View the cart and click 'more details'. Confirm authors show as
normal.
9. Click 'send' and confirm the email sent shows the authors as normal.
10. Go to the list you added the record to and click 'send list'.
Confirm the email sent shows the authors as normal.
11. View the record detail page and confirm the authors show as normal.
12. Confirm tests pass:
- t/Biblio.t
- t/db_dependent/Koha/Biblio.t

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Comment 3 David Nind 2021-02-24 06:24:33 UTC
Created attachment 117239 [details] [review]
Bug 27266: Move GetMarcAuthors to Koha namespace

This patch moves C4::Biblio::GetMarcAuthors to
Koha::Biblio->get_authors_from_MARC. This is so the method can be
used in templates and notices.

To test:
1. Find a record that has an author in the added entry field (700-720).
2. Add the record to the cart and a list.
3. View your cart and click 'more details'. Confirm authors show as
normal.
4. Click 'send' and confirm the email sent shows the authors as normal.
5. Go to the list you added the record to and click 'send list'. Confirm
the email sent shows the authors as normal.
6. Set the OPACXSLTDetailsDisplay system preference to empty (for no
xslt)
7. Log in to the OPAC. Find the record and add it to the cart and a list
8. View the cart and click 'more details'. Confirm authors show as
normal.
9. Click 'send' and confirm the email sent shows the authors as normal.
10. Go to the list you added the record to and click 'send list'.
Confirm the email sent shows the authors as normal.
11. View the record detail page and confirm the authors show as normal.
12. Confirm tests pass:
- t/Biblio.t
- t/db_dependent/Koha/Biblio.t

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2021-02-24 06:43:08 UTC
Notes from testing with koha-testing-docker:

- I couldn't send email (it comes up with an error message when you click on the send buttons - problem sending email). I didn't have this issue when testing bug 3150, so I applied that and everything worked as expected with the test plan.

- To review the email I copied the HTML from the database (koha-mysql kohadev, select * from message_queue;) and saved it as an HTML file.
Comment 5 Martin Renvoize 2021-03-16 11:24:00 UTC
Sorry, this no longer applies.
Comment 6 Martin Renvoize 2021-03-16 11:31:45 UTC
Created attachment 118311 [details] [review]
Bug 27266: Move GetMarcAuthors to Koha namespace

This patch moves C4::Biblio::GetMarcAuthors to
Koha::Biblio->get_authors_from_MARC. This is so the method can be
used in templates and notices.

To test:
1. Find a record that has an author in the added entry field (700-720).
2. Add the record to the cart and a list.
3. View your cart and click 'more details'. Confirm authors show as
normal.
4. Click 'send' and confirm the email sent shows the authors as normal.
5. Go to the list you added the record to and click 'send list'. Confirm
the email sent shows the authors as normal.
6. Set the OPACXSLTDetailsDisplay system preference to empty (for no
xslt)
7. Log in to the OPAC. Find the record and add it to the cart and a list
8. View the cart and click 'more details'. Confirm authors show as
normal.
9. Click 'send' and confirm the email sent shows the authors as normal.
10. Go to the list you added the record to and click 'send list'.
Confirm the email sent shows the authors as normal.
11. View the record detail page and confirm the authors show as normal.
12. Confirm tests pass:
- t/Biblio.t
- t/db_dependent/Koha/Biblio.t

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2021-03-16 11:35:33 UTC
Created attachment 118312 [details] [review]
Bug 27266: Move GetMarcAuthors to Koha namespace

This patch moves C4::Biblio::GetMarcAuthors to
Koha::Biblio->get_authors_from_MARC. This is so the method can be
used in templates and notices.

To test:
1. Find a record that has an author in the added entry field (700-720).
2. Add the record to the cart and a list.
3. View your cart and click 'more details'. Confirm authors show as
normal.
4. Click 'send' and confirm the email sent shows the authors as normal.
5. Go to the list you added the record to and click 'send list'. Confirm
the email sent shows the authors as normal.
6. Set the OPACXSLTDetailsDisplay system preference to empty (for no
xslt)
7. Log in to the OPAC. Find the record and add it to the cart and a list
8. View the cart and click 'more details'. Confirm authors show as
normal.
9. Click 'send' and confirm the email sent shows the authors as normal.
10. Go to the list you added the record to and click 'send list'.
Confirm the email sent shows the authors as normal.
11. View the record detail page and confirm the authors show as normal.
12. Confirm tests pass:
- t/Biblio.t
- t/db_dependent/Koha/Biblio.t

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2021-03-16 11:35:52 UTC
Rebased
Comment 9 Martin Renvoize 2021-03-16 11:37:29 UTC
Tests pass, QA scripts pass and the code looks reasonable.

I'm not entirely sure about the method name.. we now have:

Koha::Biblio->get_marc_notes;

and

Koha::Biblio->get_authors_from_MARC;

For consistency, I think I'd prefer this patch to introduce

Koha::Biblio->get_marc_authors;

But... I think we can leave that decision to the RM.. so Passing QA :)
Comment 10 Marcel de Rooy 2021-06-25 08:59:10 UTC
(In reply to Martin Renvoize from comment #9)
> Tests pass, QA scripts pass and the code looks reasonable.
> 
> I'm not entirely sure about the method name.. we now have:
> 
> Koha::Biblio->get_marc_notes;
> 
> and
> 
> Koha::Biblio->get_authors_from_MARC;
> 
> For consistency, I think I'd prefer this patch to introduce
> 
> Koha::Biblio->get_marc_authors;
> 
> But... I think we can leave that decision to the RM.. so Passing QA :)

I would also prefer get_marc_authors.
Comment 11 Aleisha Amohia 2021-07-21 21:25:41 UTC
Created attachment 123017 [details] [review]
Bug 27266: Move GetMarcAuthors to Koha namespace

This patch moves C4::Biblio::GetMarcAuthors to
Koha::Biblio->get_authors_from_MARC. This is so the method can be
used in templates and notices.

To test:
1. Find a record that has an author in the added entry field (700-720).
2. Add the record to the cart and a list.
3. View your cart and click 'more details'. Confirm authors show as
normal.
4. Click 'send' and confirm the email sent shows the authors as normal.
5. Go to the list you added the record to and click 'send list'. Confirm
the email sent shows the authors as normal.
6. Set the OPACXSLTDetailsDisplay system preference to empty (for no
xslt)
7. Log in to the OPAC. Find the record and add it to the cart and a list
8. View the cart and click 'more details'. Confirm authors show as
normal.
9. Click 'send' and confirm the email sent shows the authors as normal.
10. Go to the list you added the record to and click 'send list'.
Confirm the email sent shows the authors as normal.
11. View the record detail page and confirm the authors show as normal.
12. Confirm tests pass:
- t/Biblio.t
- t/db_dependent/Koha/Biblio.t

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Aleisha Amohia 2021-07-21 21:25:45 UTC
Created attachment 123018 [details] [review]
Bug 27266: (QA follow-up) Rename to get_marc_authors
Comment 13 Aleisha Amohia 2021-07-21 22:16:31 UTC
Created attachment 123019 [details] [review]
Bug 27266: (follow-up) Remove instances of GetMarcAuthors
Comment 14 Martin Renvoize 2021-07-22 07:48:18 UTC
Created attachment 123026 [details] [review]
Bug 27266: Move GetMarcAuthors to Koha namespace

This patch moves C4::Biblio::GetMarcAuthors to
Koha::Biblio->get_authors_from_MARC. This is so the method can be
used in templates and notices.

To test:
1. Find a record that has an author in the added entry field (700-720).
2. Add the record to the cart and a list.
3. View your cart and click 'more details'. Confirm authors show as
normal.
4. Click 'send' and confirm the email sent shows the authors as normal.
5. Go to the list you added the record to and click 'send list'. Confirm
the email sent shows the authors as normal.
6. Set the OPACXSLTDetailsDisplay system preference to empty (for no
xslt)
7. Log in to the OPAC. Find the record and add it to the cart and a list
8. View the cart and click 'more details'. Confirm authors show as
normal.
9. Click 'send' and confirm the email sent shows the authors as normal.
10. Go to the list you added the record to and click 'send list'.
Confirm the email sent shows the authors as normal.
11. View the record detail page and confirm the authors show as normal.
12. Confirm tests pass:
- t/Biblio.t
- t/db_dependent/Koha/Biblio.t

Sponsored-by: Bibliotheksservice-Zentrum Baden-Württemberg (BSZ)
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2021-07-22 07:48:22 UTC
Created attachment 123027 [details] [review]
Bug 27266: (QA follow-up) Rename to get_marc_authors

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2021-07-22 07:48:25 UTC
Created attachment 123028 [details] [review]
Bug 27266: (follow-up) Remove instances of GetMarcAuthors

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2021-07-22 08:00:16 UTC
Comment on attachment 123026 [details] [review]
Bug 27266: Move GetMarcAuthors to Koha namespace

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

::: Koha/Biblio.pm
@@ +870,5 @@
> +    # tagslib useful only for UNIMARC author responsibilities
> +    my $tagslib;
> +    if ( $marcflavour eq "UNIMARC" ) {
> +        # FIXME : we don't have the framework available, we take the default framework. May be buggy on some setups, will be usually correct.
> +        $tagslib = C4::Biblio::GetMarcStructure( 1, '', { unsafe => 1 });

I wonder if we aught to fix the FIXME here?

$tagslib = C4::Biblio::GetMarcStructure( 1, $self->frameworkcode, { unsafe => 1 });
Comment 18 Aleisha Amohia 2021-07-22 20:27:46 UTC
(In reply to Martin Renvoize from comment #17)
> Comment on attachment 123026 [details] [review] [review]
> Bug 27266: Move GetMarcAuthors to Koha namespace
> 
> Review of attachment 123026 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/Biblio.pm
> @@ +870,5 @@
> > +    # tagslib useful only for UNIMARC author responsibilities
> > +    my $tagslib;
> > +    if ( $marcflavour eq "UNIMARC" ) {
> > +        # FIXME : we don't have the framework available, we take the default framework. May be buggy on some setups, will be usually correct.
> > +        $tagslib = C4::Biblio::GetMarcStructure( 1, '', { unsafe => 1 });
> 
> I wonder if we aught to fix the FIXME here?
> 
> $tagslib = C4::Biblio::GetMarcStructure( 1, $self->frameworkcode, { unsafe
> => 1 });

True, may as well. Follow-up on it's way.
Comment 19 Aleisha Amohia 2021-07-22 20:30:51 UTC
Created attachment 123067 [details] [review]
Bug 27266: (follow-up) Using biblio frameworkcode to get rid of FIXME