Skip to content

Build put route#24

Merged
nbarnabee merged 3 commits intomainfrom
build_put_route
Feb 21, 2023
Merged

Build put route#24
nbarnabee merged 3 commits intomainfrom
build_put_route

Conversation

@nbarnabee
Copy link
Collaborator

Have added a throughput for PUT requests, passing data received from the frontend to Toolhub server.

@nbarnabee
Copy link
Collaborator Author

Now fully functional, will update our records upon a successful insertion to Toolhub. I look forward to demo'ing this at our team meeting in half an hour.

@nbarnabee nbarnabee marked this pull request as ready for review February 15, 2023 08:31
@nbarnabee
Copy link
Collaborator Author

Another force push to deal with rebasing. Side note: git reset has changed my life.

@nbarnabee nbarnabee force-pushed the build_put_route branch 2 times, most recently from 903b3b5 to 1068177 Compare February 16, 2023 15:18
Base automatically changed from add_backend_oauth to main February 16, 2023 15:19
@blancadesal
Copy link
Member

blancadesal commented Feb 20, 2023

To avoid major merge conflicts, I'm thinking it would probably be easiest to merge #29 first, then manually rebase this PR on top? What do you think @nbarnabee?

@nbarnabee
Copy link
Collaborator Author

To avoid major merge conflicts, I'm thinking it would probably be easiest to merge #29 first, then manually rebase this PR on top? What do you think @nbarnabee?

I agree entirely!

@nbarnabee
Copy link
Collaborator Author

To avoid major merge conflicts, I'm thinking it would probably be easiest to merge #29 first, then manually rebase this PR on top? What do you think @nbarnabee?

I agree entirely!

And the same is probably true of #27

@nbarnabee nbarnabee marked this pull request as draft February 20, 2023 11:12
@nbarnabee nbarnabee marked this pull request as ready for review February 20, 2023 13:52
@nbarnabee
Copy link
Collaborator Author

To my utter astonishment, this still works perfectly and is ready for review.

To test, run both the front and backend containers, and, from the toolhunt directory, upgrade and insert the mock data:

docker-compose exec flask-web python manage.py insert_fields
docker-compose exec flask-web python manage.py insert_mock_data

Go to localhost:8082/api/login and follow the authentication route, then to localhost:8082/api/documentation

I've pared the mock data down to just what I need to test this route, as it all gets completely changed by #30 anyway.
There should only be two tasks in the task table, and you can check them by making a GET request to /api/tasks
By making PUT requests to /api/tasks/{id} you can play around with the various responses given if a task doesn't exist, or has already been completed, or if the request body doesn't match the given task, etc.

You can get "success" results by entering the following request along with the correct task id.

{
"tool": "testy-mc-test-tool",
"value": "https://www.example.com",
"field": "bugtracker_url"
}

Then you can check the demo server audit logs to see the result: https://toolhub-demo.wmcloud.org/api-docs#get-/api/auditlogs/

@blancadesal
Copy link
Member

Recheck

@nbarnabee
Copy link
Collaborator Author

Force pushed rebase from main.

Copy link
Collaborator

@HWaruguru HWaruguru left a comment

Choose a reason for hiding this comment

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

Works well

@nbarnabee
Copy link
Collaborator Author

Now that the new mock data has been incorporated, my testing instructions above are out-of-date. Follow the db population instructions given in the README, which will produce plenty of sample incomplete tasks that you can test.

Removed extraneous comments, a print() function and improved docstrings.
@nbarnabee
Copy link
Collaborator Author

624a048 and 2797427 address changes suggested by @Damilare1

The main PUT route could definitely use some refactoring and better error handling. Once #31 is merged I will move many (most? all??) of these functions to ToolhubClient

Copy link
Collaborator

@Damilare1 Damilare1 left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. LGTM!

@nbarnabee nbarnabee merged commit cee18c4 into main Feb 21, 2023
@nbarnabee nbarnabee deleted the build_put_route branch February 21, 2023 13:06
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.

4 participants