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

Add dummy Detail contributor operation for show page #72

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nirnaeth
Copy link

@nirnaeth nirnaeth commented Oct 1, 2020

I am not sure this is exactly what is requested, but I'm trying to follow the instructions left in issue #20

I assumed the intent of the first request is to duplicate the current implementation for the List operation for contributors and have something similar for the show page.
If someone could confirm that, I'd be grateful.

expose :contributor

def call
@contributor = nil
Copy link
Author

Choose a reason for hiding this comment

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

I am unsure what to return in this case, I'm not extremely familiar with Hanami projects :)

@nirnaeth nirnaeth changed the title Add dummy Detail contributor peration for show pag Add dummy Detail contributor operation for show page Oct 6, 2020
@nirnaeth
Copy link
Author

nirnaeth commented Oct 8, 2020

@davydovanton just to confirm, am I going in the right direction?

@cllns
Copy link
Member

cllns commented Oct 8, 2020

I don't think #20 is calling for the extraction of a Detail interactor. I think it's calling to extract a List interator (which already exists as file, though it's un-used and empty). You can do that by moving the implementation of retrieving the contributors from the action to that interactor, and add robust tests for the interactor.

If you're looking to learn more about interactors, I recommend checking out the guide for them: https://github.com/hanami/guides/blob/master/content/architecture/interactors.md

Is that helpful?

@nirnaeth
Copy link
Author

I think it is. I will try to do that and get back to you.

@jodosha jodosha changed the base branch from master to main June 17, 2021 08:41
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.

2 participants