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

Post request #22

Merged
merged 6 commits into from Oct 17, 2021
Merged

Post request #22

merged 6 commits into from Oct 17, 2021

Conversation

tvaroglu
Copy link
Collaborator

@tvaroglu tvaroglu commented Oct 17, 2021

  • Changes Implemented:

    • Refactor api.urls.py to dry up logic (keep api/v1 namespacing within Django project root vs app module)
    • Refactor api.views.py to use Class-based views (vs Function-based views in prior implementation)
    • Add the following endpoints with a fully functional Postman suite to test the implementation of the following:
      • GET /api/v1/stories (index of all stories, note, still more work to be done here once geoloc API is integrated)
      • GET /api/v1/stories/<id:pk> (retrieve an individual story)
      • POST /api/v1/stories (create a new story)
      • DEL /api/v1/stories/<id:pk> (delete an individual story)
    • Add Postman environment vars for Dev and Prod into postman dir, so you can import both and toggle between the two, for future testing phases
    • Update PR Template to add check / step to update Postman collection (if any changes made to endpoints that could break tests as currently written).
  • Quality Control Checklist:

    • >= 20% test coverage
    • Postman collection updated (if any changes to endpoints)
    • Last Commit passes CircleCI build
  • Blockers (if applicable): None, but a special thank you to the Django docs for the incorrect tutorial on how to build routes!! 🤦🏽‍♂️

  • Next Steps & Additional Notes:

    • Complete PATCH / PUT endpoint to update a story
    • Integrate 3rd Party Geoloc API to complete the stories 'index' endpoint implementation
    • Continue to add to Postman suite and write pytest request specs (if they exist). Admittedly, have done zero research on this, and have just been using Postman to get our endpoints tested and deployed as quickly as possible for the FE team.

@tvaroglu tvaroglu moved this from In-Progress to Ready for Review in MIB Oct 17, 2021
@tvaroglu tvaroglu linked an issue Oct 17, 2021 that may be closed by this pull request
"header": [],
"body": {
"mode": "raw",
"raw": "{\n \"title\":\"Wie Traurig\",\n \"message\":\"Ich habe meinen Freund im Denver verloren und jetzt bin Ich ganz allein :(\",\n \"name\":\"Till Lindemann\",\n \"latitude\":125.456892,\n \"longitude\":-18.982791\n}",
Copy link
Contributor

Choose a reason for hiding this comment

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

Sprichst du deutsch...? I'll be your new friend :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nur ein bisschen.. leide habe Ich viel vergessen 😅

" pm.expect(attributes['latitude']).not.to.be.null;",
" pm.expect(attributes['longitude']).not.to.be.null;",
" pm.expect(attributes['message']).not.to.be.null;",
" pm.expect(attributes['name']).not.to.be.null;",

Choose a reason for hiding this comment

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

Should name be included in the post? Just want to clarify so we are sending the correct info to the POST

Choose a reason for hiding this comment

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

Also - is this a required field or is it a field where if the user does not fill it out it will send to the server as anonymous?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The latter - it is optional and defaults to Anonymous if not provided

" pm.expect(attributes['name']).not.to.be.null;",
" pm.expect(attributes['title']).not.to.be.null;",
" pm.expect(attributes['location']).not.to.be.null;",
"});"
Copy link
Contributor

Choose a reason for hiding this comment

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

Right now the Object we are sending
looks like
const newStory = {title, message, latitude, longitude}

Copy link
Contributor

@justincanthony justincanthony left a comment

Choose a reason for hiding this comment

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

Thank you for getting this going! 🚀
I commented about the object you need for the post request, but it looks like you have already addressed it in slack!

MIB automation moved this from Ready for Review to In-Progress Oct 17, 2021
Copy link
Collaborator

@matt-kragen matt-kragen left a comment

Choose a reason for hiding this comment

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

Really excellent work here, sir!

@matt-kragen
Copy link
Collaborator

Possibility to revisit in future in case we want JSON response to conform to JSON 1.0 Spec

@matt-kragen matt-kragen merged commit d5aea85 into main Oct 17, 2021
MIB automation moved this from In-Progress to Done Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
MIB
Done (Deployed)
4 participants