[Koha-bugs] [Bug 11175] Show the parent record's component parts in the detailed views

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11175

--- Comment #426 from Martin Renvoize <martin.renvoize at ptfs-europe.com> ---
(In reply to Jonathan Druart from comment #425)
> Some patches have been squashed for readability.
> 
> (In reply to Marcel de Rooy from comment #390)
> >  14 files changed, 322 insertions(+), 55 deletions(-)
> > 
> > $showcomp variable naming; show_components better
> 
> Too late, too complicated or we add a separate commit for that. Not a big
> deal IMO.

I was trying to be light-touch when I first adopted this bug.. but happy to
change this too.

> 
> > Changes XSLTParse4Display; no tests. (Yes, there is no, but there should be.)
> 
> Still missing, I will try and provide some a bit later today.

Damn, I meant to add these tests.

> 
> > Code block if ($xslsyspref =~ m/Details/) {
> > We are fetching components while we may not use them.
> > Have the impression that the if inside the block should be incorporated into
> > the main condition.

We use them for the 'Show analytics' link handling stuff.. I was also not a fan
of always firing this query.. but that predates this patchset.

> (In reply to Marcel de Rooy from comment #394)
> > Not from this report, but very related:
> > 
> >         $self->{_components} = $results if ( defined($results) &&
> > scalar(@$results) );
> >     }
> > 
> >     return $self->{_components} || [];
> > 
> > Why do we save these component parts in the object and not use them later
> > (if we would call again)?
> 
> Yes, that's a bad pattern. We discussed that recently but cannot remember
> where.
> We shouldn't cache that in the object IMO.

The caching here appears to be completely broken to me.. likely by one of the
many many rebases/follow-ups. The idea was that we execute this search from
both C4::XSLT (for the 'Show analytics' link handling) AND each of the details
controllers.. caching at the object level would mean we could re-use the
results from the controller level at the XSLT call.  I'm not sure how else you
could cache this if not at the object level.

-- 
You are receiving this mail because:
You are watching all bug changes.