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

187434306 set up flow to create extension #46

Closed
wants to merge 21 commits into from

Conversation

zee6197
Copy link

@zee6197 zee6197 commented Apr 24, 2024

General Info

Changes

Explain your changes here (in such a way that you would understand why you made them a year from now).

Set up the flow for creating extension along with rspec test coverage,

  • Added extensions controller
  • Added extensions controller spec.rb

Testing

Explain how you tested your changes. If testing is not necessary, explain why.

Tested the changes:

spec/controllers/api/v1/extensions_controller_spec.rb

Documentation

Does this PR require documentation. If so, explain where it can be found.

Documentation is in the group meeting documentation google doc.

Checklist

  • Name of branch corresponds to story

Copy link
Member

Choose a reason for hiding this comment

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

Nit: we already have a gem for mocking out http requests. You can see my test file for the facade to see how that's done.

Copy link
Member

Choose a reason for hiding this comment

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

Don't include your .DS_Store. It should also be in your .gitignore

Copy link
Member

Choose a reason for hiding this comment

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

Nit: Ideally avoid including comments for things you plan on doing in the future -- the correct place for that is on the tracker as a separate ticket.

def destroy
render :json => 'not yet implemented'.to_json, status: 501
render body: 'The index method of CoursesController is not yet implemented'.to_json, status: 501
Copy link
Member

Choose a reason for hiding this comment

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

Nit: try to stay within convention for how we're handling unimplemented routes. We have a standard response payload.

Copy link
Member

Choose a reason for hiding this comment

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

For all of these, we should use standard (updated) ruby syntax for hashes (use : instead of =>

end


#TODO: Test get/delete
Copy link
Member

Choose a reason for hiding this comment

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

Same thing as before with the commented code / todo

@ekandell ekandell mentioned this pull request Apr 29, 2024
2 tasks
@Connor-Bernard
Copy link
Member

Included in #47 with necessary fixes.

@Connor-Bernard Connor-Bernard deleted the 187434306-set-up-flow-to-create-extension branch April 30, 2024 00:26
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