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

feat: APP-352 buying options filter #2575

Open
wants to merge 17 commits into
base: dev
Choose a base branch
from

Conversation

blushi
Copy link
Member

@blushi blushi commented Jan 8, 2025

Description

https://regennetwork.atlassian.net/browse/APP-352
Also closes: APP-501


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • provided a link to the relevant issue or specification
  • provided instructions on how to test
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

How to test

From https://deploy-preview-2575--regen-marketplace.netlify.app/projects/1:
Test the new buying options filters.

Selected buying options filters should impact the "credit class" filter options:

  • when selecting "credit card", only credit classes with projects with some fiat sell orders or "unregistered projects" if there are prefinance projects should appear
  • when selecting "crypto", "unregistered projects" option should be hidden

Selected credit classes should impact the "buying options" filter options:

  • when selecting "unregistered projects" only, "crypto" option should be hidden if there are some prefinance projects
  • when all selected credit classes have no fiat sell orders, "credit card" option should be hidden.

Check that on prefinance projects page, there's only the "buying options" filter with "credit card" checked as the only option and disabled.

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items
.

I have...

  • confirmed all author checklist items have been addressed
  • reviewed code correctness and readability
  • verified React components follow DRY principles
  • reviewed documentation is accurate
  • reviewed tests
  • manually tested (if applicable)

Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for regen-website ready!

Name Link
🔨 Latest commit bf32505
🔍 Latest deploy log https://app.netlify.com/sites/regen-website/deploys/677fc9e88ec36200087aca0d
😎 Deploy Preview https://deploy-preview-2575--regen-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for terrasos ready!

Name Link
🔨 Latest commit bf32505
🔍 Latest deploy log https://app.netlify.com/sites/terrasos/deploys/677fc9e85cf2fa00081aa4c5
😎 Deploy Preview https://deploy-preview-2575--terrasos.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@blushi blushi marked this pull request as draft January 9, 2025 08:37
@blushi blushi requested a review from r41ph January 9, 2025 12:52
@blushi blushi marked this pull request as ready for review January 9, 2025 12:52
Comment on lines +113 to +114
enabled:
!isOffChainProject && !classId && !projectId && !!ecocreditClient,
Copy link
Member Author

@blushi blushi Jan 9, 2025

Choose a reason for hiding this comment

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

unrelated to this task but just a small optimization I spotted: in case of using this on an offchain project page, we were running getProjectsQuery, which is not useful here, because in the case of an offchain projectId (= on chain project id) is not defined so enabled was true

@@ -10,6 +10,7 @@ import { client as sanityClient } from 'lib/clients/sanity';
import { AnchoredProjectMetadataLD } from 'lib/db/types/json-ld';
Copy link
Member Author

Choose a reason for hiding this comment

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

important changes to review in this file

@blushi
Copy link
Member Author

blushi commented Jan 9, 2025

@erikalogie @S4mmyb please see testing instructions above

@erikalogie
Copy link
Collaborator

@blushi looks great, here is some feedback:

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.

2 participants