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

Contributor edit endpoint #11

Open
wants to merge 4 commits into
base: contributor-show-endpoint
Choose a base branch
from

Conversation

leoncalermo
Copy link

Summary

  • Contributor update endpoint

  • with tests

  • login will be done after along with pundiit

Screenshots

[Change!] Show the screenshots of the views you modified.

Trello Card

Comment on lines 11 to 13
def render_error(status, message)
render json: message, status: status
end
Copy link
Contributor

Choose a reason for hiding this comment

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

You are not using this method, we could delete it


def update
contributor = Contributor.find(params[:id])
return render json: contributor, status: :ok if contributor.update(update_params)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the status: :ok bit is not needed, maybe you can check which response code gives if you delete that?

class ContributorPolicy < ApplicationPolicy
attr_reader :user, :contributor

def initialize(user, post)
Copy link
Contributor

@marcegl91 marcegl91 Apr 21, 2020

Choose a reason for hiding this comment

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

It seems that this class is not needed, provided that we don't have a Post model in this project.

Copy link
Contributor

@marcegl91 marcegl91 left a comment

Choose a reason for hiding this comment

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

Great work! 💯

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