-
-
Notifications
You must be signed in to change notification settings - Fork 797
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
Query returns null for the image field of the likedBy object of the post. #2507
Conversation
WalkthroughThe changes include an update to the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PostsResolver
participant Database
Client->>PostsResolver: Request posts
PostsResolver->>Database: Query posts
Database-->>PostsResolver: Return posts data
PostsResolver->>Database: Populate likedBy images, firstName, lastName
Database-->>PostsResolver: Return populated data
PostsResolver-->>Client: Return posts with user details
Possibly related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- src/resolvers/Organization/posts.ts (1 hunks)
Additional comments not posted (1)
src/resolvers/Organization/posts.ts (1)
82-85
: LGTM!The code changes correctly populate the
likedBy
field with theimage
property, fixing the issue of theimage
field returning null for users who liked the post.
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- src/resolvers/Organization/posts.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/resolvers/Organization/posts.ts
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2507 +/- ##
========================================
Coverage 98.77% 98.77%
========================================
Files 355 355
Lines 17992 17996 +4
Branches 2400 2400
========================================
+ Hits 17772 17776 +4
Misses 220 220 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- src/resolvers/Organization/posts.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/resolvers/Organization/posts.ts
@pranshugupta54 can you please review it sir? |
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.
Please add a test to ensure that this error doesn't repeat itself.
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.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (8)
- .coderabbit.yaml (1 hunks)
- INSTALLATION.md (1 hunks)
- README.md (1 hunks)
- docker-compose.prod.yaml (1 hunks)
- scripts/cloud-api-demo/README.md (1 hunks)
- tests/helpers/userAndOrg.ts (1 hunks)
- tests/resolvers/Post/likedBy.spec.ts (1 hunks)
- vite.config.mts (1 hunks)
Files skipped from review due to trivial changes (6)
- .coderabbit.yaml
- INSTALLATION.md
- README.md
- docker-compose.prod.yaml
- scripts/cloud-api-demo/README.md
- vite.config.mts
Additional comments not posted (1)
tests/resolvers/Post/likedBy.spec.ts (1)
17-38
: Setup and Teardown Functions: Well ImplementedThe
beforeAll
andafterAll
functions are correctly implemented using async/await to handle asynchronous operations, ensuring that the database connection is properly managed before and after tests.
tests/resolvers/Post/likedBy.spec.ts
Outdated
describe("resolvers -> organization -> posts", () => { | ||
it(`returns the post object for parent post`, async () => { | ||
const parent = await Organization.findById(testOrganization?._id).lean(); | ||
|
||
if (!parent) { | ||
throw new Error("Parent organization not found."); | ||
} | ||
|
||
const postPayload = (await postResolver?.(parent, { first: 1 }, {})) as { | ||
edges: { node: InterfacePost }[]; | ||
totalCount: number; | ||
}; | ||
|
||
expect(postPayload).toBeDefined(); | ||
if (!postPayload) { | ||
throw new Error("postPayload is null or undefined"); | ||
} | ||
expect(postPayload.edges).toBeDefined(); | ||
expect(Array.isArray(postPayload.edges)).toBe(true); | ||
|
||
const posts = await Post.find({ | ||
organization: testOrganization?._id, | ||
}).lean(); | ||
|
||
console.log("postPayloadedge:", postPayload.edges[0].node.likedBy[0]); | ||
console.log("posts:", posts[0].likedBy[0]); | ||
|
||
expect(postPayload.edges.length).toEqual(posts.length); | ||
expect(postPayload.totalCount).toEqual(posts.length); | ||
const returnedPost = postPayload.edges[0].node; | ||
expect(returnedPost._id).toEqual(testPost?._id.toString()); | ||
expect(returnedPost.likedBy).toHaveLength(1); | ||
expect(returnedPost.likedBy[0]._id).toEqual(testUser?._id); | ||
expect(returnedPost.likedBy[0].image).not.toBeNull(); | ||
}); |
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.
Test Case Review: Comprehensive but Consider Removing Debug Logs
The test case is well-structured and checks important aspects of the likedBy
functionality. However, consider removing or commenting out the debug logs at lines 64-65 for cleaner test output in production environments.
tests/helpers/userAndOrg.ts
Outdated
@@ -24,7 +24,7 @@ export const createTestUser = async (): Promise<TestUserType> => { | |||
password: `pass${nanoid().toLowerCase()}`, | |||
firstName: `firstName${nanoid().toLowerCase()}`, | |||
lastName: `lastName${nanoid().toLowerCase()}`, | |||
image: null, | |||
image: "exampleimageurl.com", |
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.
Modification in createTestUser
: Appropriate for Testing
The addition of a default image URL in the createTestUser
function is a suitable change for testing scenarios involving user images. Consider documenting this default value in the function's JSDoc to inform other developers of its purpose and usage.
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- tests/resolvers/Post/likedBy.spec.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/resolvers/Post/likedBy.spec.ts
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.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- tests/resolvers/Post/likedBy.spec.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/resolvers/Post/likedBy.spec.ts
@palisadoes I have added a test, this is the first time I have written a test so if you have any practices that I should follow, please share so that i can update this and keep that in mind for my next PR's, thank you sir. |
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.
Please make the suggested changes
.coderabbit.yaml
Outdated
@@ -15,4 +15,4 @@ reviews: | |||
- develop | |||
- main | |||
chat: | |||
auto_reply: true |
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.
Your PR has too many unnecessary files that don’t relate to your issue. You can from the PR by running the commands below from the root directory of the repository
git add -u
git reset HEAD path/to/file
git commit
Please apply these changes to this file.
INSTALLATION.md
Outdated
@@ -324,8 +324,8 @@ Follow these steps for setting up a software development environment. | |||
``` | |||
2. Running asynchronously in a subshell. You will have to use the `docker-compose down` command below to stop it. | |||
`bash | |||
sudo /usr/libexec/docker/cli-plugins/docker-compose -f docker-compose.dev.yaml up --build & | |||
` | |||
sudo /usr/libexec/docker/cli-plugins/docker-compose -f docker-compose.dev.yaml up --build & |
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.
Your PR has too many unnecessary files that don’t relate to your issue. You can from the PR by running the commands below from the root directory of the repository
git add -u
git reset HEAD path/to/file
git commit
Please apply these changes to this file.
README.md
Outdated
@@ -44,11 +44,11 @@ Core features include: | |||
|
|||
1. You can install the software for this repository using the steps in our [INSTALLATION.md](INSTALLATION.md) file. | |||
1. Do you want to contribute to our code base? Look at our [CONTRIBUTING.md](CONTRIBUTING.md) file to get started. There you'll also find links to: | |||
1. Our code of conduct documentation in the [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md) file. | |||
1. How we handle the processing of new and existing issues in our [ISSUE_GUIDELINES.md](ISSUE_GUIDELINES.md) file. | |||
1. The methodologies we use to manage our pull requests in our [PR_GUIDELINES.md](PR_GUIDELINES.md) file. |
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.
Your PR has too many unnecessary files that don’t relate to your issue. You can from the PR by running the commands below from the root directory of the repository
git add -u
git reset HEAD path/to/file
git commit
Please apply these changes to this file.
scripts/cloud-api-demo/README.md
Outdated
@@ -237,4 +237,3 @@ echo "0 * * * * talawa-api python3 reset_database.py --mongo-container develop-m | |||
### 8.3 Logging for cron jobs | |||
|
|||
This will set up logging for the cron jobs and manage log rotation using logrotate. |
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.
Your PR has too many unnecessary files that don’t relate to your issue. You can from the PR by running the commands below from the root directory of the repository
git add -u
git reset HEAD path/to/file
git commit
Please apply these changes to this file.
vite.config.mts
Outdated
@@ -52,9 +52,9 @@ export default defineConfig({ | |||
testTimeout: 30000, | |||
|
|||
// Use a thread pool for parallel execution to improve performance | |||
pool: 'threads', |
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.
Your PR has too many unnecessary files that don’t relate to your issue. You can from the PR by running the commands below from the root directory of the repository
git add -u
git reset HEAD path/to/file
git commit
Please apply these changes to this file.
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.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- README.md (2 hunks)
- vite.config.mts (2 hunks)
Additional context used
Markdownlint
README.md
30-30: Expected: 2; Actual: 0
Inconsistent indentation for list items at the same level(MD005, list-indent)
Additional comments not posted (2)
vite.config.mts (1)
13-13
: LGTM!The removal of the
pool
property is approved.README.md (1)
30-30
: LGTM!The adjustments to the indentation of the numbered list items are approved. The changes improve the formatting and readability of the documentation.
Tools
Markdownlint
30-30: Expected: 2; Actual: 0
Inconsistent indentation for list items at the same level(MD005, list-indent)
README.md
Outdated
@@ -27,7 +27,7 @@ Core features include: | |||
|
|||
- [Talawa Components](#talawa-components) | |||
- [Documentation](#documentation) | |||
- [Videos](#videos) | |||
- [Videos](#videos) |
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.
Fix the indentation of the list items.
Markdownlint indicates inconsistent indentation for list items at the same level. Please ensure that all list items at the same level have consistent indentation.
Apply the following change to fix the indentation:
-1. You can install the software for this repository using the steps in our [INSTALLATION.md](INSTALLATION.md) file.
-1. Do you want to contribute to our code base? Look at our [CONTRIBUTING.md](CONTRIBUTING.md) file to get started. There you'll also find links to:
+ 1. You can install the software for this repository using the steps in our [INSTALLATION.md](INSTALLATION.md) file.
+ 1. Do you want to contribute to our code base? Look at our [CONTRIBUTING.md](CONTRIBUTING.md) file to get started. There you'll also find links to:
1. Our code of conduct documentation in the [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md) file.
1. How we handle the processing of new and existing issues in our [ISSUE_GUIDELINES.md](ISSUE_GUIDELINES.md) file.
1. The methodologies we use to manage our pull requests in our [PR_GUIDELINES.md](PR_GUIDELINES.md) file.
-1. The `talawa` documentation can be found at our [docs.talawa.io](https://docs.talawa.io) site.
+ 1. The `talawa` documentation can be found at our [docs.talawa.io](https://docs.talawa.io) site.
1. It is automatically generated from the markdown files stored in our [Talawa-Docs GitHub repository](https://github.com/PalisadoesFoundation/talawa-docs). This makes it easy for you to update our documenation.
Committable suggestion was skipped due to low confidence.
Tools
Markdownlint
30-30: Expected: 2; Actual: 0
Inconsistent indentation for list items at the same level(MD005, list-indent)
Please make sure that coderabbit.ai approves your PR. Make the suggested changes. |
yeah i am on it |
We have a policy of unassigning contributors who close PRs without getting validation from our reviewer team. This is because:
Please be considerate of our volunteers' limited time and our desire to improve our code base. This policy is stated as a pinned post in all our Talawa repositories. Our YouTube videos explain why this practice is not acceptable to our Community. Unfortunately, if this continues we will have to close the offending PR and unassign you from the issue. |
am really sorry for this, I will remember this next time, won't happen again |
the reviews suggested by coderabbit.ai are outdated as the files have been unstaged |
@palisadoes: I will review the changes. Actions performedReview triggered.
|
Hi @palisadoes I have fixed all errors related to the PR, I just need some help to fix this linting error, as I have a failing test, I tried check with prettier but it showed everything was good, I even ran This happening because the setup.ts file as a eslint-disable comment, when i removed the comment i recieved several errors 10:1 error './src/constants' import is restricted from being used by a pattern no-restricted-imports can you please tell me what am I missing here |
Please ask the #talawa-api slack channel for assistance |
I actually did ask this on slack, but nobody replied, that's why I tagged you, I will ask this again on slack. |
@pranshugupta54 @varshith257 Do you have any suggestions? |
What kind of change does this PR introduce?
bugfix
Issue Number:
#2262
Fixes #
Did you add tests for your changes?
Yes
Snapshots/Videos:
If relevant, did you update the documentation?
No
Summary
This solves the issue of null image of users that had liked posts.
Does this PR introduce a breaking change?
No, it's a bug fix
Other information
Have you read the contributing guide?
yes
Summary by CodeRabbit
Summary by CodeRabbit
likedBy
field, such as user images, first names, and last names, improving the richness of information available to users.