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.

Sign up for GitHub

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

Jump to bottom

feat: Project detail page #605

Merged
merged 44 commits into from
Sep 30, 2024
Merged

feat: Project detail page #605

merged 44 commits into from
Sep 30, 2024

Conversation

ZibanPirate
Copy link
Member

@ZibanPirate ZibanPirate commented Sep 22, 2024 •
edited
Loading

  • revamped projects listing page
  • added new project detail page

see #607

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (changes that require a fresh clone of the repo)
  • I'm not sure

@github-actions github-actions bot added data Data related changes ( data folder ) web Web related changes ( web folder ) api API related changes ( api folder ) labels Sep 22, 2024
ZibanPirate
ZibanPirate commented Sep 22, 2024
View reviewed changes
api/src/app/middlewares/security.ts Outdated Show resolved Hide resolved
api/src/project/repository.ts Show resolved Hide resolved
Base automatically changed from cloudflare-redirect to main September 22, 2024 19:20
@github-actions github-actions bot added models models package utils utils package labels Sep 23, 2024
@ZibanPirate
navigate to project page
d3841c9
@ZibanPirate
mvp for project page
c4e8883
@ZibanPirate
project title on project page
f7f6a2f
@ZibanPirate
added back button
14337f0
@ZibanPirate
added project stats
0cd0218
@ZibanPirate
don't rate limit in development
4d7e91d
@ZibanPirate
added repos and contors
bee976a
@ZibanPirate
show open contributions of project
7301626
@ZibanPirate
return project stats via api
ac9cff2
@ZibanPirate
adjust project ranking to account for stars
b46d35a
@ZibanPirate
return project repositories via api
91ebbc9
@ZibanPirate
return project contributors via api
a3e8c54
@ZibanPirate
return project contributions via api
1b2a7bd
@ZibanPirate
remove debug code
d93341b
@ZibanPirate
fix lhci server shutdown while at it
a00efa4
@ZibanPirate
responsive design for project detail on mobile app
91aa3ef
@ZibanPirate
only show contributions of a project when there are some
f01ca86
@ZibanPirate
relaxed rate limit
cbc25f5
@ZibanPirate
show stats in projects page
868a33c
@ZibanPirate
style tweaks for projects page
d48d080
@ZibanPirate
setup cloudflare functions
bbd55ec
@ZibanPirate
first dynamic cloudflare route
30eab4f
@github-actions github-actions bot added the repo label Sep 28, 2024
@ZibanPirate ZibanPirate mentioned this pull request Sep 30, 2024
Open
3 tasks
@github-actions github-actions bot added the feature Feature label Sep 30, 2024
@ZibanPirate ZibanPirate marked this pull request as ready for review September 30, 2024 17:39
omdxp
omdxp reviewed Sep 30, 2024
View reviewed changes
api/db/migrations/0000_melodic_shotgun.sql
@@ -39,6 +50,7 @@ CREATE TABLE `repositories` (
`name` text NOT NULL,
`run_id` text DEFAULT 'initial-run-id' NOT NULL,
`project_id` integer NOT NULL,
`stars` integer NOT NULL,
FOREIGN KEY (`project_id`) REFERENCES `projects`(`id`) ON UPDATE no action ON DELETE no action
Copy link
Member

Choose a reason for hiding this comment

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

can we add a readme column here?

Copy link
Member Author

Choose a reason for hiding this comment

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

what for? if for showing it in the project detail page, it will require more effort, we need to:

  • fetch the readme file and save it in the db in the crown job.
  • update the UI, and account for multiple repos case.

Copy link
Member

@omdxp omdxp Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

how about we fetch the readme file from backend and we static generate the readme section in project details? no need to save it in db

Copy link
Member Author

Choose a reason for hiding this comment

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

that's another way of doing it, thoe we will rich the Github rate-limit which is the reason why we had to switch to the cron-job, feel free to explore how we implement this in a separate task, but for now this will do 👌

omdxp
omdxp reviewed Sep 30, 2024
View reviewed changes
api/db/migrations/0001_black_eternals.sql
Copy link
Member

Choose a reason for hiding this comment

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

why this migration file was deleted?

Copy link
Member Author

Choose a reason for hiding this comment

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

two reasons:

  • SQLite doesn't support altering tables, we may need to switch to another db engine later.
  • our data is refreshed on every cron job run, deleting the database and starting over is not a problem

Copy link
Member

Choose a reason for hiding this comment

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

it'd be better to have versioning in db structure that relates to the api version (some people might only need the api), that's why it is better to keep migrations

omdxp
omdxp reviewed Sep 30, 2024
View reviewed changes
api/db/migrations/meta/0001_snapshot.json
Copy link
Member

Choose a reason for hiding this comment

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

same here?

Copy link
Member Author

Choose a reason for hiding this comment

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

same reason mentioned here:
#605 (comment)

omdxp
omdxp reviewed Sep 30, 2024
View reviewed changes
package.json
@@ -9,6 +9,7 @@
"@types/node": "^20",
"@types/semver": "^7.3.9",
"cross-env": "^7.0.3",
"del-cli": "^5.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

what's the benefit of using this instead of rimraf?

Copy link
Member Author

Choose a reason for hiding this comment

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

for security reasons, rimraf doesn't support glob pattern when ran via cli (to not delete the entire system), del-cli does support it

@ZibanPirate ZibanPirate merged commit a7c6a52 into main Sep 30, 2024
23 checks passed
@ZibanPirate ZibanPirate deleted the project-detail branch September 30, 2024 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omdxp omdxp omdxp approved these changes

@souhaib-benbouzid souhaib-benbouzid Awaiting requested review from souhaib-benbouzid

Assignees
No one assigned
Labels
api API related changes ( api folder ) data Data related changes ( data folder ) feature Feature models models package repo utils utils package web Web related changes ( web folder )
Projects
DzCode i/o
Status: Merged
Milestone
Project, Contribution, Contributor
Development

Successfully merging this pull request may close these issues.
2 participants
@ZibanPirate @omdxp