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

Only display tasks with price labels (or without, depending on env) #77

Closed
wants to merge 16 commits into from

Conversation

xno-miner
Copy link

Resolves ubiquity/.github#115

Filter issues with price label when loading issue previews in src/home/fetch-github/fetch-issues-preview.ts.

@xno-miner
Copy link
Author

This part is done (it works for me locally at least).

The amount of issues that can be fetched with a single request is 30 (default) or 100 (max). There are currently 92 issues in the devpool (including those without price label) so I implemented pagination to avoid future problems with the website.

package.json Outdated Show resolved Hide resolved
@ubiquity-os-deployer
Copy link

ubiquity-os-deployer bot commented Aug 15, 2024

xno-miner and others added 4 commits August 17, 2024 20:57
Co-authored-by: アレクサンダー.eth <4975670+0x4007@users.noreply.github.com>
@xno-miner
Copy link
Author

I added an RFC enviromental variable. For the false value it works just as before. It normally wouldn't work for true value yet, but it almost does, because of the issues i accidentaly created. The only problem is it doesn't display any labels. This is becase the issues it's displaying don't have any (they're the ones i accidentally added with my bot). For the labels to be displayed, all issues in devpool-directory created by testbot6 need to be removed. After that, once the backend will be merged and the issues will be synced, the labels will be displayed correctly.

@xno-miner xno-miner changed the title Only display tasks with price labels Only display tasks with price labels (or without, depending on env) Aug 17, 2024
@rndquu
Copy link
Member

rndquu commented Aug 19, 2024

@xno-miner
Copy link
Author

@xno-miner
Copy link
Author

@xno-miner Could you fix the tests

Seems like the tests were also failing in the main branch and have been fixed here: f14181a. I have added this commit to the branch.

@xno-miner xno-miner marked this pull request as ready for review August 26, 2024 22:04
@xno-miner xno-miner requested a review from 0x4007 August 26, 2024 22:06
@0x4007
Copy link
Member

0x4007 commented Aug 27, 2024

@rndquu do you mind seeing this rfc project reviews through?

@0x4007 0x4007 removed their request for review August 27, 2024 02:39
@rndquu rndquu self-requested a review August 27, 2024 15:18
Copy link
Member

@rndquu rndquu left a comment

Choose a reason for hiding this comment

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

@xno-miner

  1. Pls resolve code conflicts
  2. Getting this error when setting RFC=true in the .env file:
Screenshot 2024-08-27 at 18 26 44

Pls fix

@@ -29,6 +29,7 @@ export const esBuildContext: esbuild.BuildOptions = {
SUPABASE_STORAGE_KEY: generateSupabaseStorageKey(),
commitHash: execSync(`git rev-parse --short HEAD`).toString().trim(),
NODE_ENV: process.env.NODE_ENV || "development",
IS_RFC: JSON.parse(process.env.RFC!)
Copy link
Member

Choose a reason for hiding this comment

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

It makes sense to provide a default value here in case process.env.RFC is empty, like process.env.RFC || false. This way we make it backwards compatible with already deployed version of work.ubq.fi.

Copy link
Author

Choose a reason for hiding this comment

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

cypress/e2e/devpool.cy.ts Outdated Show resolved Hide resolved
@@ -71,7 +77,7 @@ export async function fetchIssuePreviews(): Promise<TaskNoFull[]> {
}
}

const tasks = freshIssues.map((preview: GitHubIssue) => ({
const tasks = freshIssues.filter((issue: GitHubIssue) => (issue.labels as GitHubLabel[]).some((label) => label.name.includes("Pricing")) == !IS_RFC).map((preview: GitHubIssue) => ({
Copy link
Member

Choose a reason for hiding this comment

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

How IS_RFC gets a value here? I mean that we define it here but don't assign it a value anywhere. Is it working?

Copy link
Author

Choose a reason for hiding this comment

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

env variables are passed in at build time

source: comments at the top of src/home/rendering/render-github-login-button.ts

@xno-miner
Copy link
Author

2. Getting this error when setting `RFC=true` in the `.env` file:
Screenshot 2024-08-27 at 18 26 44

Pls fix

The same error exists in the deployed version on https://work.ubq.fi. The error exists because some issue links include www. which causes the regex to not match. I will create a pull request in this repository separate from this one so it can be merged separately quicker.

@xno-miner
Copy link
Author

The same error exists in the deployed version on https://work.ubq.fi. The error exists because some issue links include www. which causes the regex to not match. I will create a pull request in this repository separate from this one so it can be merged separately quicker.

I have created the PR: #86

@xno-miner
Copy link
Author

Still waiting for review, @rndquu.
Here, as well as in ubiquity/devpool-directory#1250

.env.example Outdated Show resolved Hide resolved
@0x4007
Copy link
Member

0x4007 commented Sep 25, 2024

Is the idea to fork this repo and run rfc.ubq.fi off of it? They will need to be hosted on two separate organizations for a fork to be possible.

I realize we can't QA test this with our deployer because you're setting RFC to false in the build script.

Deploys: #77 (comment)

.github/workflows/build.yml Outdated Show resolved Hide resolved
@0x4007
Copy link
Member

0x4007 commented Sep 25, 2024

QA Test Results

Context

I merged the "backend" changes (to sync every issue basically) and now work.ubq.fi currently displays everything, even if they don't have prices.

image

Requested Changes

  • This makes me think: the environment variable approach seems wrong.
  • Why don't you just add a checkbox toggle/filter "Tasks" and when clicked it switches to "Proposals" or something?
  • This is intended to hide all the issues without a Price label.

"Backend" Delta Summary

58 open ➡️ 143 open...

Run npx tsx ./scripts/delete-unauthorized-issues.ts
  
Deleting issue https://github.com/ubiquity/devpool-directory/issues/1[10](https://github.com/ubiquity/devpool-directory/actions/runs/11041825775/job/30672939774#step:8:10)9 created by bot ubiquibot[bot]
Deleting issue https://github.com/ubiquity/devpool-directory/issues/817 created by bot ubiquibot[bot]
Deleting issue https://github.com/ubiquity/devpool-directory/issues/816 created by bot ubiquibot[bot]
Deleting issue https://github.com/ubiquity/devpool-directory/issues/815 created by bot ubiquibot[bot]
Deleting issue https://github.com/ubiquity/devpool-directory/issues/741 created by bot ubiquibot[bot]
Deleting issue https://github.com/ubiquity/devpool-directory/issues/589 created by bot ubiquibot[bot]

137 open net result.

Screenshot

It looks like it works as expected with RFC env var! How did you test/develop this? Perhaps you could have shown us screenshots and proof on your end? Apologies for the slow review period but that certainly would have sped things along!

e93b0680 devpool-directory-ui pages dev_

Remarks

Archived Repositories

@0x4007 0x4007 closed this Sep 26, 2024
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 support for proposals on work.ubq.fi
4 participants