Link to TPAC library pages from copy table in results and KPAC for consistency

Bug #1271600 reported by Dan Scott
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

* Evergreen master

Per Dan Wells' comment in #1261939 and mirroring #1201982 we should be consistent about where we're linking to on the search results copy table.

Thus, please merge the top commit in;a=shortlog;h=refs/heads/user/dbs/tpac_library_results

Dan Wells (dbw2)
Changed in evergreen:
milestone: 2.6.0-alpha1 → 2.6.0-beta1
Revision history for this message
Dan Scott (denials) wrote :

Removing pullrequest tag until I address the KPAC copy table as well. (3 copy tables, really?)

tags: removed: pullrequest
Revision history for this message
Dan Scott (denials) wrote :

I've pushed some updates to this branch. Going deeper down the KPAC rabbit hole, I refactored some of the library page bits so we could call them from both TPAC and KPAC contexts without too much code duplication. It's working now, although you lose the KPAC breadcrumb context when you jump to the library page (because we're not maintaining the GET params in the link to the library page). Maybe look at REFERER as a cleaner way of pulling the previous pages' params?

Revision history for this message
Dan Scott (denials) wrote :

I also added some reasonable/minimal CSS so the KPAC library info page didn't look ridiculous. At this point, I think it's worth putting the pullrequest tag back in effect. This branch improves significantly over what is currently in master.

summary: - Link to TPAC library pages from copy table in results for consistency
+ Link to TPAC library pages from copy table in results and KPAC for
+ consistency
Revision history for this message
Ben Shum (bshum) wrote :

The KPAC side looked very nice to me Dan. Thanks for your hard work on that piece of it.

Couple issues though for the regular TPAC -- the record and result (with more details) views were broken with apache Internal Server Errors. Looking at my logs I found this:

open-ils.cstore 2014-01-24 23:54:03 [ERR :3345:oils_sql.c:5585:13906237963958117] open-ils.cstore: Error with query [SELECT * FROM actor.org_unit_ancestor_setting( 'lib.info_url', '' ) AS "actor.org_unit_ancestor_setting" ;]: 0 ERROR: invalid input syntax for integer: ""
LINE 1: ... actor.org_unit_ancestor_setting( 'lib.info_url', '' ) AS "a...

Digging deeper, I found that when we pulled out the code that makes the URLs into its separate file, there was a move to make "copy_info = copy;" but this was not present in the results/copy_table.tt2 file (which broke all more detailed results). Unfortunately, the record one was also broken, because it was backwards, and it really wanted "copy = copy_info;"

I've signed off on the commits in the branch, and added a new commit at the top to address the variables issue.

See branch: user/bshum/tpac_library_results;a=shortlog;h=refs/heads/user/bshum/tpac_library_results

tags: added: pullrequest
Changed in evergreen:
status: New → Confirmed
importance: Undecided → Wishlist
Revision history for this message
Dan Scott (denials) wrote :

Thank you Ben! Clearly my head got too focused on sorting out KPAC; I'm sure I tried manually testing TPAC after the refactoring, but might have just been getting cached results from hitting "reload" instead of a fresh search. No excuse, really... but thank you!

Pushed to master.

Dan Scott (denials)
Changed in evergreen:
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers