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

Rename get and create factories API to ohshown-event #46

Merged

Conversation

EagleC0318
Copy link
Collaborator

Why

What

  • Rename the API endpoint url

How

  • Rename the API endpoint {host}/server/api/factories to {host}/server/api/ohshown-events
  • Fix the unit test.

Test

  • Pass unit test
  • Sanity check with the FE locally (modified locally to call the new API endpoint), looks good.
    image

Copy link

@tai271828 tai271828 left a comment

Choose a reason for hiding this comment

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

Wow the sprint today is really productive!! Thanks for the effort and I am so excited about this very first step of #44.

I am +1 to this pull request, but I don't have a practical idea if we should land it now or later. Does FE want to follow up this change at the same time, or let's land anyway and let FE follow up later? @Neilxx @yanghaochang104 which approach you prefer?

@tai271828 tai271828 requested a review from Neilxx March 13, 2022 18:54
@EagleC0318
Copy link
Collaborator Author

Wow the sprint today is really productive!! Thanks for the effort and I am so excited about this very first step of #44.

I am +1 to this pull request, but I don't have a practical idea if we should land it now or later. Does FE want to follow up this change at the same time, or let's land anyway and let FE follow up later? @Neilxx @yanghaochang104 which approach you prefer?
@tai271828 I can also commit my FE local change for testing this PR, and create another PR for FE repo.

@tai271828
Copy link

tai271828 commented Mar 14, 2022

@tai271828 I can also commit my FE local change for testing this PR, and create another PR for FE repo.

Wooooow that's really greate! So let me wait for your PR for FE repo then? Thanks! Let's try to land both of them (PR of BE/FE) at the same time.

@EagleC0318
Copy link
Collaborator Author

@tai271828 I can also commit my FE local change for testing this PR, and create another PR for FE repo.

Wooooow that's really create! So let me wait for your PR for FE repo then? Thanks! Let's try to land both of them (PR of BE/FE) at the same time.

@tai271828 FE PR created!

@tai271828
Copy link

OH-SHOWN/ohshown-frontend#63 landed, so let's move forward together.

@tai271828 tai271828 merged commit 3bfd3e4 into OH-SHOWN:main-tbbca Mar 15, 2022
@tai271828
Copy link

Deployed to the development site. Everything looks good.

By "looks good" I meant manual checks pass including:

[1] initial loading

System check identified no issues (0 silenced).
March 15, 2022 - 17:31:31                             
Django version 2.2.13, using settings 'gis_project.settings'
Starting development server at http://0.0.0.0:8000/                                                                   
Quit the server with CONTROL-C.                                                                                       
"GET /api/ohshown-events?range=5&lng=120.48504632216294&lat=24.088258816482295 HTTP/1.1" 200 28344

[2] communication between when uploading images

System check identified no issues (0 silenced).
March 15, 2022 - 17:31:31                             
Django version 2.2.13, using settings 'gis_project.settings'
Starting development server at http://0.0.0.0:8000/                                                                   
Quit the server with CONTROL-C.                                                                                       
"GET /api/ohshown-events?range=5&lng=120.48504632216294&lat=24.088258816482295 HTTP/1.1" 200 28344

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

2 participants