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

Feature: sprint checkin form submission status for user #149

Merged
merged 8 commits into from
May 14, 2024

Conversation

Ajen07
Copy link
Collaborator

@Ajen07 Ajen07 commented May 8, 2024

Description

added check in submission status in the response, the sprintId will be in sprintCheckin list if the user has submitted a check in form for the sprint

Issue link


Fixes # (issue)

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?

  • unit tests
    unit-test-passed

  • e2e tests
    e2e-test-passed

  • swagger
    swaager

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

@Ajen07 Ajen07 self-assigned this May 8, 2024
Copy link
Collaborator

@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.

edit: move to review comment

Copy link
Collaborator

@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.

we shouldn't need to add any field to the table, we can retrieve the data without added another field to the table

you can retrieve the data from sprint check in table and append it to the /me endpoint

we don't want to add extra/unnecessary columns to tables, we need to make use of the relations

I guess one thing to keep in mind is we don't want to create duplicate data just for the purpose of retrieval.

====
So to reiterate,

in the fetch user detail endpoint users/me,

fetch the users voyageTeamMemberId, use this id to query sprintCheckin table, append this data to the response, so something like this

prisma.FormResponseCheckin.findMany({
where: voyageTeamMemberId {
id: { in: voyageTeamMemberId}
}, seleect:{
  sprintId: true
}

where voyageTeamMemberId is the list of Id from the user table

image

@Ajen07 Ajen07 requested a review from cherylli May 8, 2024 07:01
JoshuaHinman
JoshuaHinman previously approved these changes May 10, 2024
Copy link
Contributor

@JoshuaHinman JoshuaHinman left a comment

Choose a reason for hiding this comment

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

Tests passed and checkins displayed in response - nice job! It would have been helpful for the users/me route to be identified in the Description or Tested section - at first I assumed it was the sprints/checkin route

@cherylli
Copy link
Collaborator

sorry I forgot to submit my reviews which I did a couple of days ago, so they were pending

@Ajen07
Copy link
Collaborator Author

Ajen07 commented May 11, 2024

Tests passed and checkins displayed in response - nice job! It would have been helpful for the users/me route to be identified in the Description or Tested section - at first I assumed it was the sprints/checkin route

Got it ! .From the next time , I will make my descriptions more elaborative about the issue and its solutions .

Copy link
Collaborator

@timDeHof timDeHof left a comment

Choose a reason for hiding this comment

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

Great job! All tests passes and swagger endpoint responses with a 201.

@Ajen07 Ajen07 merged commit df5b6c5 into dev May 14, 2024
1 check passed
@Ajen07 Ajen07 deleted the feature/sprint_checkin_form_submission_status_for_user branch May 14, 2024 03:05
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

4 participants