search

sciencehistory / chf-sufia

sufia-based hydra app
Other
9 stars 4 forks source link

UrlGenerationError when there's an empty child work #665

Closed hackartisan closed 7 years ago

hackartisan commented 7 years ago

If parent work has child work which has no members, loading the parent work errors with ActionController::UrlGenerationError due to child work presenter returning nil riiif_file_id. Desired behavior is to show the empty child in the show page thumbnail list, where clicking it does not open the viewer, but not show the empty child in the viewer itself, since it cannot be viewed.

related to #617; leaving this ticket open because I'm going to merge a fix without code review; perhaps it will be reviewed later.

jrochkind commented 7 years ago

@jrochkind should review code before close.

jrochkind commented 7 years ago

I gotta admit I don't completely understand the changes from b8f199e and aac5748 (those were what I was invited to look at?), but I don't see any obvious red flags. @hackmastera is there anything in particular you weren't sure about and wanted another pairs of eyes on? Otherwise I'll just go ahead and close, bigger fish to fry!

hackartisan commented 7 years ago

@jrochkind ah, sorry about that; I should have linked the PR which explains the 2 commits: https://github.com/chemheritage/chf-sufia/pull/666

The first (b8f199e) didn't really work, mostly demonstrated a different approach which tried to put more logic in presenters and less in views. So don't worry about that unless you hated the other one.

The second aac5748 was the fix I went ahead and pushed.

Nothing in particular I hoped you would look at, just didn't want to go sneaking around code review :)