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

Return a default presenter instead of nil. #2117

Merged
merged 1 commit into from
Oct 29, 2019

Conversation

dkinzer
Copy link
Member

@dkinzer dkinzer commented Jun 27, 2019

The presenter helper method should return a default presenter instead of
nil. Otherwise an error occurs when you have a new action that is
not one of "show", "citation", or "index".

The `presenter` helper method should return a default presenter instead of
nil.  Otherwise an error occurs when you have a new action that is
not one of "show", "citation", or "index".
@jcoyne
Copy link
Member

jcoyne commented Jul 3, 2019

I don't understand why you'd want an IndexPresenter for something that isn't an index view. Can you tell me why that would be what you want?

@dkinzer
Copy link
Member Author

dkinzer commented Jul 9, 2019

So in this particular case I'm rendering something via ajax using some index view partials. So it works, but I guess my problem is more with the fact that we are returning nil and then later in some views assuming the this command does not return nil.

I'm not sure why this wasn't failing for me when we were on BL-6 but now it is.

I think at the very least this method should not be returning nil ever.

Maybe there should be way for an action to register a specific presenter?

If you think this is not useful I can close this PR and just continue to override in our instance.

@bess
Copy link
Member

bess commented Oct 29, 2019

I think this looks like a helpful change and I'd like to merge it.

@bess bess merged commit 008ec7f into projectblacklight:master Oct 29, 2019
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.

None yet

3 participants