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

Update/sprints add voyage dates #147

Merged
merged 6 commits into from
May 10, 2024
Merged

Conversation

JoshuaHinman
Copy link
Contributor

@JoshuaHinman JoshuaHinman commented May 6, 2024

Description

Updates response for route GET /api/v1/voyages/sprints/teams/{teamId} - adds startDate and endDate to voyage properties

-also added soloProjectDeadline, certificateIssueDate, showcasePublishDate to response body

Issue link

Fixes task 86b026m0d

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature updates / changes
  • Tests
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

yarn test:e2e sprints

Tested in swagger
image

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have updated the change log

@JoshuaHinman JoshuaHinman self-assigned this May 6, 2024
cherylli
cherylli previously approved these changes May 8, 2024
Copy link
Contributor

@cherylli cherylli left a comment

Choose a reason for hiding this comment

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

everything works fine but I found out that the endpoint 200 sample response doesn't match the actual response, maybe we can update it in another PR

CHANGELOG.md Outdated Show resolved Hide resolved
@JoshuaHinman
Copy link
Contributor Author

everything works fine but I found out that the endpoint 200 sample response doesn't match the actual response, maybe we can update it in another PR

I noticed that too - I could follow up with another revision

Co-authored-by: Cheryl M <webmaster@cherylli.com>
@cherylli
Copy link
Contributor

cherylli commented May 8, 2024

everything works fine but I found out that the endpoint 200 sample response doesn't match the actual response, maybe we can update it in another PR

I noticed that too - I could follow up with another revision

sure if you don't mind, I've also added another task for that on clickup so if you decide to do it you can claim that and add to this, otherwise happy to just merge this first

Ajen07
Ajen07 previously approved these changes May 8, 2024
Copy link
Contributor

@Ajen07 Ajen07 left a comment

Choose a reason for hiding this comment

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

All changes looks good . It passes all the unit and e2e tests

@cherylli cherylli self-requested a review May 8, 2024 23:55
cherylli
cherylli previously approved these changes May 8, 2024
@JoshuaHinman JoshuaHinman dismissed stale reviews from cherylli and Ajen07 via 9281e97 May 9, 2024 07:04
@JoshuaHinman
Copy link
Contributor Author

everything works fine but I found out that the endpoint 200 sample response doesn't match the actual response, maybe we can update it in another PR

I noticed that too - I could follow up with another revision

sure if you don't mind, I've also added another task for that on clickup so if you decide to do it you can claim that and add to this, otherwise happy to just merge this first

I've updated the response to match the example, and fixed a test that broke

Copy link
Contributor

@cherylli cherylli left a comment

Choose a reason for hiding this comment

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

thanks. I think you changed the actually response instead of the sample response, but this works too, I think they won't need solo project date but the other 2 dates could be useful in future

@JoshuaHinman
Copy link
Contributor Author

thanks. I think you changed the actually response instead of the sample response, but this works too, I think they won't need solo project date but the other 2 dates could be useful in future

That's funny - yeah I assumed it should go the other way, but whatever you think is fine

@cherylli cherylli requested a review from Ajen07 May 10, 2024 04:11
@JoshuaHinman JoshuaHinman merged commit fdbf79d into dev May 10, 2024
1 check passed
@JoshuaHinman JoshuaHinman deleted the update/sprints-add-voyage-dates branch May 10, 2024 21:49
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