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

implement new hustle api call #903

Merged
merged 18 commits into from
Dec 8, 2023

Conversation

nmannes
Copy link
Contributor

@nmannes nmannes commented Oct 9, 2023

This is my first PR, so i figured I'd do something pretty basic. I just implemented an additional API call in the Hustle module in accordance with their docs that I found in something tagged as a good first issue

@shaunagm shaunagm self-requested a review October 25, 2023 18:19
Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

Hi @nmannes! Welcome to Parsons! (Are you in the Slack? We'd love to have you)

Sorry for the delay in responding to this PR. I'm also sorry the tests didn't run automatically - I've fixed the settings so first-time contributors can still run tests. :)

From the tests, it looks like there's an issue with linting. You can follow the instructions here to fix it - let me know if you have any questions.

The only other issue is that we'll want a test for this new method. You can copy/base your test on existing tests in the test_hustle.py file. Again, let me know if you have any questions. Writing tests can be a little confusing, especially if you're unfamiliar with mocks. We've got a guide to using mocks in tests here. :)

@shaunagm
Copy link
Collaborator

Hi @nmannes what's the status of this? Would you like help with the tests? Happy to do a pairing session

@nmannes
Copy link
Contributor Author

nmannes commented Nov 16, 2023

Hi @nmannes what's the status of this? Would you like help with the tests? Happy to do a pairing session

Hi @shaunagm! Thank you for following up. Yes, I would like to go over how to write a test for this with you. Do you have a calendly or something like that?

@shaunagm
Copy link
Collaborator

Yes, I do have a cal.com link - I DMed it to you on the parsons slack

@shaunagm shaunagm added the 🎉 first PR the first PR by a new contributor label Nov 21, 2023
@nmannes
Copy link
Contributor Author

nmannes commented Nov 21, 2023

Update: Shauna and I spoke and there was an issue trying to generate a hustle oath2 token from sandbox credentials. Will follow up after the holidays to see why that was no good

@shaunagm
Copy link
Collaborator

shaunagm commented Dec 8, 2023

After a bit of emailing back and forth we got credentials and were able to test this manually against a sandbox, so we're good to merge! :)

@shaunagm shaunagm merged commit 4008447 into move-coop:main Dec 8, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 first PR the first PR by a new contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants