Bug 9740 - using XSLT modifies record authorized values
Summary: using XSLT modifies record authorized values
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: 3.14
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 7706
  Show dependency treegraph
 
Reported: 2013-03-01 15:23 UTC by Fridolin Somers
Modified: 2020-11-30 21:45 UTC (History)
4 users (show)

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


Attachments
Proposed patch (2.22 KB, patch)
2013-03-01 15:35 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 9740: using XSLT modifies record authorized values (2.28 KB, patch)
2013-03-19 13:55 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2013-03-01 15:23:54 UTC
When using XSLT display, in detail.pl, the record (MARC::Record) is get from database.
This record is given to XSLTParse4Display() to transform into HTML using XSLT.
This method uses transformMARCXML4XSLT() to replace in record authorized values code by there description.
The bug is that this change is made on original record and remains in the rest of the script detail.pl.
Comment 1 Fridolin Somers 2013-03-01 15:35:06 UTC Comment hidden (obsolete)
Comment 2 Galen Charlton 2013-03-12 22:40:10 UTC
I agree with your analysis.  However, I do have a question: have you seen an user-visible consequences?
Comment 3 Fridolin Somers 2013-03-19 08:12:35 UTC
(In reply to comment #2)
> I agree with your analysis.  However, I do have a question: have you seen an
> user-visible consequences?

A library has an authorized value defined on a 300$a (UNIMARC).
That is how I saw the bug and composed the test plan.
Comment 4 Paul Poulain 2013-03-19 13:55:21 UTC
Patch tested with a sandbox, by Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 5 Paul Poulain 2013-03-19 13:55:43 UTC
Created attachment 16393 [details] [review]
Bug 9740: using XSLT modifies record authorized values

When using XSLT display, in detail.pl, the record (MARC::Record) is get from database.
This record is given to XSLTParse4Display() to transform into HTML using XSLT.
This method uses transformMARCXML4XSLT() to replace in record authorized values code by there description.
The bug is that this change is made on original record and remains in the rest of the script detail.pl.

This patch adds a clonning operation into transformMARCXML4XSLT.

Test plan :
- Activate XSLT display in intranet details XSLTDetailsDisplay syspref
- Create a new authorized value categorie : ie "AUTHVAL"
- Add a new authorized value and description : ie "VAL" and "Description for VAL"
- Edit default framework
- Edit a note subfield : 3xx for UNIMARC, 5xx for others) : ie 326$a
- Set authorized value "AUTHVAL" for this subfield
- Edit a biblio with default framework
- Choose authorized value for speficied subfield : ie "Description for VAL" for 326$a
- Go to detail page and click on "Descriptions" tab
=> Whitout this patch you see authorized value description : ie "Description for VAL"
=> Whith this patch you see authorized value code : ie "VAL"
"Descriptions" tab is set in detail.pl with C4::GetMarcNotes($record,$marcflavour) after the call to XSLTParse4Display($record), so test plan confirms that XSLTParse4Display() modifies or not $record.

Signed-off-by: Sonia Bouis <sonia.bouis@univ-lyon3.fr>
Comment 6 Katrin Fischer 2013-03-31 10:59:10 UTC
Hi Fridolyn,
I am not sure I understand the problem here. I cataloged a record using authorised values for 500 (footnote) and 653 (subject). The code was "QA".

In OPAC and staff all views show my definded description "Quality Assurance". 
When I export the record, it says "QA" in both fields.

When I edit the record, both pull downs are preselected correctly.

This is on master without your patch. Can you please explain?
Comment 7 Fridolin Somers 2013-04-02 16:19:12 UTC
Hie Katrin.

> In OPAC and staff all views show my defined description "Quality Assurance". 
With your test, you should see that the code is displayed when you turn XSLT off.

Do you see the description in MARC detail page ?
Comment 8 Katrin Fischer 2013-04-02 16:20:56 UTC
Hi Fridolyn,
maybe I don't understand what the wanted/unwanted behaviour is?
Comment 9 Fridolin Somers 2013-04-04 13:22:14 UTC
(In reply to comment #8)
> Hi Fridolyn,
> maybe I don't understand what the wanted/unwanted behaviour is?

In my opinion, the unwanted behavior is that the "Descriptions" tab content depends on the use of XSLT : 
 - without XSLT, the code is displayed
 - with XSLT, the description is displayed
This is clearly a unwanted side-effect : changes on record appears in XSLT.pm but effect appears in details.pl.

The wanted behavior is that the code is always displayed.
Comment 10 Katrin Fischer 2013-04-04 14:05:54 UTC
I think I disagree. We have authorized values with a special description for the OPAC - so that is what display. We also use authorised values for local subjectes (653) where the library does not want to create real authorities but have some control over the terms. So in that case we always want to display the long description and not the short code.
Comment 11 Jared Camins-Esakov 2013-04-04 14:52:24 UTC
I think that the current behavior is a feature not a bug. There would have to be some sort of switch to make it possible to selectively disable the display of the label.

If the issue is that you want the labels to match the codes of a particular set of authorized values, I'd suggest just setting the labels to match the codes (or, even better, train the librarians to use meaningful labels).

Bumping back to 3.14 since we are in feature freeze.
Comment 12 Fridolin Somers 2013-04-04 15:47:45 UTC
(In reply to comment #10)
> I think I disagree. We have authorized values with a special description for
> the OPAC - so that is what display. We also use authorised values for local
> subjectes (653) where the library does not want to create real authorities
> but have some control over the terms. So in that case we always want to
> display the long description and not the short code.

The patch does not remove the conversion from code to description inside the block computed by XSLT, it removes this conversion for code outside of XSLT.pm
Comment 13 Fridolin Somers 2013-04-04 15:49:32 UTC
Actually the side effect is only visible in fields shown in "Description" tab, that is what I describe in test plan.
Comment 14 Fridolin Somers 2013-04-04 15:57:18 UTC
(In reply to comment #11)
> I think that the current behavior is a feature not a bug.
This can't be a feature, changes on var $record performed in XSLT.pm had effects in details.pl.
Comment 15 Jared Camins-Esakov 2013-04-04 16:05:17 UTC
(In reply to comment #14)
> (In reply to comment #11)
> > I think that the current behavior is a feature not a bug.
> This can't be a feature, changes on var $record performed in XSLT.pm had
> effects in details.pl.

No question that the code is poor, but surely it can be fixed without breaking functionality?
Comment 16 Fridolin Somers 2013-06-13 08:44:14 UTC
(In reply to comment #15)
> No question that the code is poor, but surely it can be fixed without
> breaking functionality?
Just in case : I do not speak about the authorized value conversion in the block displayed by the XSLT.

I think no one add noticed that side effect and used it has a functionality.

If you wan, I create a follow-up that replaces in "Description" tab the authorized values by there description.
Comment 17 Fridolin Somers 2013-07-17 13:46:42 UTC
I found a side effect of this bug : Bug 7706
Comment 18 Owen Leonard 2016-06-23 14:12:38 UTC
Is this bug still valid? If so, should this discussion be restarted?
Comment 19 Fridolin Somers 2020-03-30 11:31:57 UTC
Looks like there is no side-effects actually so I close