add purpose filter with trees, conservation and restoration #485
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request modifies the Changes
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
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/Layout/QueryParamContext.tsx (1)
241-241: Consider making the purpose filter configurable.While the current implementation fixes the immediate issue, consider making the purpose filter values configurable through environment variables or a configuration object. This would provide more flexibility for future changes without code modifications.
Example implementation:
- url: `/app/projects?_scope=map&filter[purpose]=trees,restoration,conservation`, + url: `/app/projects?_scope=map&filter[purpose]=${config.allowedPurposes || 'trees,restoration,conservation'}`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Layout/QueryParamContext.tsx(1 hunks)
🔇 Additional comments (2)
src/Layout/QueryParamContext.tsx (2)
Line range hint
241-259: Implementation looks good!The changes maintain proper error handling and data processing while adding the purpose filter. The existing functionality for filtering allowed donations and random project selection is preserved.
241-241: Change aligns well with PR objectives.The implementation successfully addresses the API endpoint issues by adding the requested purpose filter for trees, conservation, and restoration. This serves as an effective quick fix while maintaining the possibility for future improvements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/eslint.yml (1)
15-18: Document Node.js version choiceThe Node.js setup looks good, using the latest action version and an LTS Node version. Consider adding a comment explaining why Node 18.x was chosen to help with future maintenance.
- name: Setup Node.js uses: actions/setup-node@v3 with: + # Using Node 18.x LTS for stability and long-term support node-version: '18.x'🧰 Tools
🪛 actionlint (1.7.4)
16-16: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/eslint.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/eslint.yml
16-16: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (1)
.github/workflows/eslint.yml (1)
21-21: Review dependency conflicts requiring --legacy-peer-depsThe use of
--legacy-peer-depssuggests there might be unresolved peer dependency conflicts in the project. While this flag works as a temporary solution, it's recommended to:
- Review the npm install logs to identify the specific conflicts
- Update dependencies to compatible versions where possible
- Document any conflicts that cannot be resolved immediately
Let's identify the specific dependency conflicts:
Additionally to @jmiridis having added some default filtering in case a filter is missing to the api endpoint to retrieve the project list as the app crashed with the latest results (e.g. not containing some tpo property for some projects), we maybe should add some filtering as well into the frontend. This is just a quick fix. Please review what kind of filter might be wanted for this application. I used the same now as done by @jmiridis in the backend. Compare this thread: https://planetfoundation.slack.com/archives/C02393H3GDC/p1736524954057429
(Also adapted faulty GitHub action for linting.)
Summary by CodeRabbit
Summary by CodeRabbit