-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Added Support for User Family Members #1713
Added Support for User Family Members #1713
Conversation
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersWhen your PR has been assigned reviewers contact them to get your code reviewed and approved via:
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
Please fix the conflicting files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have interchangeably used the term familyGroup
and Family
in the PR definitions. To make it unambiguous for future developers use userFamily
to replace familyGroup
and Family
.
import "dotenv/config"; | ||
import type mongoose from "mongoose"; | ||
import { Types } from "mongoose"; | ||
import type { MutationCreateFamilyGroupArgs } from "../../../src/types/generatedGraphQLTypes"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have interchangeably used the term familyGroup
and Family
in the PR definitions. To make it unambiguous for future developers use userFamily
to replace familyGroup
and Family
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
committed changes @palisadoes |
Please fix the failing tests |
Please fix the tests. |
working on the failing tests will fix them ASAP. |
Please fix the conflicting files |
1 similar comment
Please fix the conflicting files |
Please fix the conflicting file |
@palisadoes The PR is up for review now all the tests has been passed from my side for the code that I have changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some changes in the proposed schema, and subsequently, the implemented resolvers. Otherwise, the overall implementation looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename file to addUserToUserFamily
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Please fix the failing tests |
f7bdb60
to
8a79c3f
Compare
@palisadoes @EshaanAgg All the checks are passed from my side. Can you approve the PR now. Thanks ![]() |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #1713 +/- ##
===========================================
- Coverage 98.17% 97.91% -0.27%
===========================================
Files 184 234 +50
Lines 10767 14124 +3357
Branches 835 1175 +340
===========================================
+ Hits 10571 13829 +3258
- Misses 186 262 +76
- Partials 10 33 +23 ☔ View full report in Codecov by Sentry. |
@SiddheshKukade here is the UI equivalent of this feature. 806a668f-722e-4956-a860-693f4c61ee6c.mp4 |
8707a9c
into
PalisadoesFoundation:develop
This reverts commit 8707a9c.
You bypassed the lint rules some how and now everyone else is having linting issues. Please resubmit your changes with the correct linting applied. |
@palisadoesI haven't overridden or bypassed any of the changes; they were all pushed in the usual manner, and the checks also passed in this pull request. Could you please provide a link to a pull request where the lint checks failed, or share some resource indicating that checks failed for this reason? I'd like to take a look at why those changes failed. |
I'll hang around until you do. |
If I was hasty, I apologize. The timing was so close that I might have jumped to an incorrect conclusion |
@palisadoes so should I open a new PR. |
Yes |
@AdityaRaimec22 In most cases, the use of |
@LakshyaSatpal The code I've added isn't failing in the checks of this PR or when tested locally. Additionally, the code I've implemented draws inspiration from existing code in the codebase; I haven't introduced any new methods to incorporate this feature So I don't think |
What kind of change does this PR introduce?
Add Support for User Family Members
Issue Number:
Fixes #1584
Did you add tests for your changes?
Yes
Snapshots/Videos:
If relevant, did you update the documentation?
Not relevant
Summary
Does this PR introduce a breaking change?
No
Other information
Some tests are failing and I am currently addressing them, and I have raised a pull request because the assigned issue has been pending for a significant period.
Have you read the contributing guide?
Yes