add filter condition to remove endaoment projects from recently updated query sort #1921
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/repositories/projectRepository.ts (1)
200-204: Consider extracting the filter condition for reusability.The endaoment filter might be needed in other sorting cases in the future. Consider extracting it into a separate method like
addEndaomentFilterfor consistency and reusability:+const addEndaomentFilter = (query: SelectQueryBuilder<Project>) => { + return query.andWhere('organization.label != :label', { + label: ORGANIZATION_LABELS.ENDAOMENT, + }); +}; case SortingField.RecentlyUpdated: query .orderBy('project.updatedAt', OrderDirection.DESC) - .andWhere('organization.label != :label', { - label: ORGANIZATION_LABELS.ENDAOMENT, - }); + addEndaomentFilter(query); break;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/repositories/projectRepository.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
src/repositories/projectRepository.ts (3)
26-26: LGTM!The import of
ORGANIZATION_LABELSis correctly added and aligns with the PR objective to filter endaoment projects.
200-204: Implementation looks good!The changes:
- Correctly implement the PR objective to filter out endaoment projects
- Are properly scoped to only affect the RecentlyUpdated sorting
- Follow TypeORM query builder patterns
200-204: Verify the impact of the new filter condition.The implementation correctly adds the filter to exclude endaoment projects from recently updated results. However, let's verify:
- The existence and value of
ORGANIZATION_LABELS.ENDAOMENT- The potential impact on query performance
✅ Verification successful
The filter implementation is verified and safe to use ✅
- The
ORGANIZATION_LABELS.ENDAOMENTconstant is properly defined and consistently used across the codebase- The query performance impact is acceptable as organization filtering is lightweight
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the ENDAOMENT label constant and analyze query performance # Test 1: Check if ENDAOMENT is defined in organization.ts echo "Checking ENDAOMENT label definition:" ast-grep --pattern 'export const ORGANIZATION_LABELS = { $$$ ENDAOMENT: $_, $$$ }' # Test 2: Check if there are any other references to ENDAOMENT that might be affected echo -e "\nChecking other ENDAOMENT references:" rg -l "ENDAOMENT" --type ts # Test 3: Analyze the query performance impact echo -e "\nChecking for existing indexes on organization.label:" ast-grep --pattern 'CreateIndexOptions|@Index\('Length of output: 1622
There was a problem hiding this comment.
@divine-comedian I tested this, it's not showing Endaoment projects in "Recently updated" list. Approved ;)
fix for #1919
not sure how to test it locally
Summary by CodeRabbit