Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fix catalog searching #99

Merged
merged 1 commit into from
Jun 21, 2024
Merged

🐛 Fix catalog searching #99

merged 1 commit into from
Jun 21, 2024

Conversation

kirkkwang
Copy link
Contributor

The #truncate_and_iconify_auto_link method from
Hyrax::OverrideHelperBehavior was not being loaded. I added an include statement to the HykuKnapsack::ApplicationHelper which solved the problem, though I thought this wasn't needed.

Ref:

pals-38

The `#truncate_and_iconify_auto_link` method from
`Hyrax::OverrideHelperBehavior` was not being loaded.  I added an
include statement to the HykuKnapsack::ApplicationHelper which solved
the problem, though I thought this wasn't needed.

Ref:
  - #38
@ShanaLMoore
Copy link
Contributor

ShanaLMoore commented Jun 21, 2024

hm, I vaguely remember Rob correcting us too about not needing to include helper files. Everything in that directory should be auto loading, if I recall; that's default rails behavior for views, but we'd need to include them in non view files, like controllers.

Did you try including it in the catalog controller instead?

I'd maybe time box trying to figure out why it's not loading without the include, if that's the case.

@laritakr
Copy link
Contributor

I'd maybe time box trying to figure out why it's not loading without the include, if that's the case.

Did you try moving it into the hyku_knapsack helpers directory? I thought everything inside that directory gets autoloaded.

@kirkkwang
Copy link
Contributor Author

@ShanaLMoore I tried moving the methods in to the CatalogControllerDecorator but it didn’t pick it up
@laritakr I tried moving it into the app/helpers/hyku_knapsack/ and it also didn’t work

Copy link
Contributor

@ShanaLMoore ShanaLMoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We time boxed attempts to figure out why it's not working automatically. For the sake of time and progress, we will have to figure out why another time.

@kirkkwang kirkkwang merged commit 875d15d into main Jun 21, 2024
5 of 6 checks passed
@kirkkwang kirkkwang deleted the i38-fix-catalog-search branch June 21, 2024 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants