Bound with redesign #4070
Bound with redesign #4070
Conversation
jcoyne
force-pushed
the
redesign-bound-with
branch
6 times, most recently
from
e7faba0 to
c55e250
Compare
jcoyne
force-pushed
the
redesign-bound-with
branch
from
c55e250 to
1460c50
Compare
jcoyne
force-pushed
the
redesign-bound-with
branch
3 times, most recently
from
dcf1113 to
8b22a43
Compare
jcoyne
force-pushed
the
redesign-bound-with
branch
4 times, most recently
from
1508530 to
f5d39dc
Compare
| def bound_with_title | ||
| bound_with = item.bound_with_parent | ||
|
|
||
| # TODO: remove this after the index is rebuild with https://github.com/sul-dlss/searchworks_traject_indexer/pull/1379 |
There was a problem hiding this comment.
Looks like this was merged 2 weeks ago so the index should have been reindexed by now. Please remove these lines before merge.
|
|
||
| # TODO: remove this after the index is rebuild with https://github.com/sul-dlss/searchworks_traject_indexer/pull/1379 | ||
| return if bound_with.respond_to?(:instance) | ||
|
|
| @@ -0,0 +1,69 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
There was a problem hiding this comment.
This seems to be repeating a lot of what is in module AccessPanels.LocationItemComponent. Is there a reason we have to repeat all the same code. Could we use inheritance or something along those lines to not repeat code?
There was a problem hiding this comment.
Yes, but I believe the way to do that is by extracting a new class rather than forcing inheritance. I'm trying not to make this PR too challenging to review, thus I've avoided this at this time. Keep in mind that #4107 comes after this PR and will substantially change what is in here.
There was a problem hiding this comment.
See #4151 as a way to preemptively reduce the duplication.
lib/holdings/item.rb
Outdated
| @@ -62,6 +62,11 @@ def type | |||
| item_display[:type] | |||
| end | |||
|
|
|||
| # TODO: I believe we can look in `bound_with.hrid` for this after a reindex | |||
| def instance_hrid | |||
| item_display[:instance_hrid] | |||
There was a problem hiding this comment.
Please double check before merge.
lib/holdings/item.rb
Outdated
| # TODO: This depends on https://github.com/sul-dlss/searchworks_traject_indexer/pull/1379 | ||
| return item_display[:bound_with] if item_display[:bound_with] | ||
|
|
||
| # TODO: we're only keeping the rest of this until the index is rebuild with the PR linked above PR |
There was a problem hiding this comment.
Remove. Reindex should have happened.
jcoyne
force-pushed
the
redesign-bound-with
branch
2 times, most recently
from
8104917 to
963d3d9
Compare
From the component. This supports the split of the LocationItemComponent in #4070, so that there is no duplication
jcoyne
force-pushed
the
redesign-bound-with
branch
from
963d3d9 to
fb6495e
Compare
Ref #4074