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: add pinnedIds to useProjectsWithOrders #2191

Merged
merged 6 commits into from
Oct 31, 2023

Conversation

flagrede
Copy link
Collaborator

@flagrede flagrede commented Oct 24, 2023

Description

Closes: regen-network/rnd-dev-team#1811
Needs: regen-network/regen-sanity#49

Add the possibility to pin projects to the featured projects section on the registry homepage.
useProjectsWithOrders now accepts a pinnedIds prop to enable this feature.


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

  1. https://deploy-preview-2191--regen-marketplace.netlify.app/
  2. For devs: Try to change the featured projects in sanity staging (note that off-chain projects will need to be validated and published).

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)

@netlify
Copy link

netlify bot commented Oct 24, 2023

Deploy Preview for regen-website ready!

Name Link
🔨 Latest commit fb4a326
🔍 Latest deploy log https://app.netlify.com/sites/regen-website/deploys/6540c74831dc63000893090d
😎 Deploy Preview https://deploy-preview-2191--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.

@flagrede flagrede requested a review from a team October 24, 2023 10:02
@flagrede
Copy link
Collaborator Author

@erikalogie You can check the featured projects change here: https://deploy-preview-2191--regen-marketplace.netlify.app/
I've set up Sandy Cross as the top project on staging as an example. Unfortunately changing the feature projects on staging requires running regen-sanity locally so you won't be able to update it yourself.

@erikalogie
Copy link
Collaborator

@erikalogie You can check the featured projects change here: https://deploy-preview-2191--regen-marketplace.netlify.app/ I've set up Sandy Cross as the top project on staging as an example. Unfortunately changing the feature projects on staging requires running regen-sanity locally so you won't be able to update it yourself.

Looks great!

netlify.toml Outdated
@@ -41,7 +41,7 @@
NEXT_PUBLIC_INTERCOM_APP_ID = "kn5di10s"

[context.deploy-preview.environment]
VITE_SANITY_TAG = "default"
VITE_SANITY_TAG = "pinned-projects"
Copy link
Member

Choose a reason for hiding this comment

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

temporarily set this to a specific graphql endpoint tag so it doesn't interfere with other work
will need to set it back to default before merging

Copy link
Member

Choose a reason for hiding this comment

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

don't forget to reverse this and deploy graphql to staging default

@@ -35,6 +38,7 @@ export interface ProjectsWithOrdersProps {
projectId?: string; // to filter by project
skippedProjectId?: string; // to discard a specific project
classId?: string; // to filter by class
pinnedIds?: string[]; // list of on-chain id, uuid or slug to pinned at the top
Copy link
Member

Choose a reason for hiding this comment

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

I don't see the pinned project if I provide a slug or uuid, looks like it might not be handled yet here?

Copy link
Collaborator Author

@flagrede flagrede Oct 30, 2023

Choose a reason for hiding this comment

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

It's most likely because the project you tried to pin was not approved or published. We're using the following hook to get off-chain projects there.
Also, note that slug takes precedent over uuid: https://github.com/regen-network/regen-web/blob/dev/web-marketplace/src/lib/normalizers/projects/normalizeProjectsWithMetadata.ts#L85-L90
Could probably fix that by adding a uuid or offchainId in the normalizedObject.

Copy link
Member

@blushi blushi Oct 30, 2023

Choose a reason for hiding this comment

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

I've tried with "woodburn" as slug which is both published and approved (see data on sanity staging)

postgres=> select approved, published from project where slug='woodburn';
 approved | published 
----------+-----------
 t        | t

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Made a fix for the last part mentioned above here: 0ac4071

Copy link
Collaborator Author

@flagrede flagrede Oct 30, 2023

Choose a reason for hiding this comment

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

@blushi There was an in the commit I made earlier, which I just fixed. Also, I stopped relying on the name field and created separate ones for both uuid and slug this should ensure consistent results. Woodburn is now correctly pinned here: https://deploy-preview-2191--regen-marketplace.netlify.app/

Copy link
Member

Choose a reason for hiding this comment

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

Working now for woodburn!
Just a nit but would it be possible to open up the project page using the slug too? currently it uses the uuid which might not be the best for sharing.
But I still don't see the 2nd featured project with a uuid, although it's published and approved:

image

Copy link
Member

Choose a reason for hiding this comment

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

oh I guess that might be due to the projects on chain pagination limit...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's because this project is neither returned by the on-chain projects hooks (beyond the 100 projects current limitation) or the off-chain projects hooks (since it has an on-chain id).

Copy link
Member

Choose a reason for hiding this comment

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

yeah I figured, I'll create an issue to track the on chain queries pagination default limit related work, I guess one solution would be to query for all the pages to make sure we get all projects or credit classes...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've made the modification to use the slug whenever available here: c64154f

@flagrede flagrede requested a review from blushi October 31, 2023 08:08
Copy link
Member

@blushi blushi left a comment

Choose a reason for hiding this comment

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

tACK

netlify.toml Outdated
@@ -41,7 +41,7 @@
NEXT_PUBLIC_INTERCOM_APP_ID = "kn5di10s"

[context.deploy-preview.environment]
VITE_SANITY_TAG = "default"
VITE_SANITY_TAG = "pinned-projects"
Copy link
Member

Choose a reason for hiding this comment

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

don't forget to reverse this and deploy graphql to staging default

@flagrede flagrede merged commit 92d23c9 into dev Oct 31, 2023
11 checks passed
@flagrede flagrede deleted the feat-1811-select-featured-projects branch October 31, 2023 09:29
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.

Add ability to manually select the featured projects on app homepage
3 participants