refactor(go-website): connect list page to database - #5839
Merged
Hidden character warning
The head ref may contain hidden characters: "\ud83d\udd78\ufe0f\ud83d\udeab\ud83d\udc0d-\ud83d\udcbd2"
Conversation
michaelkedar
force-pushed
the
🕸️🚫🐍-💽2
branch
2 times, most recently
from
August 24, 2026 05:48
314405d to
31d27eb
Compare
michaelkedar
force-pushed
the
🕸️🚫🐍-💽
branch
2 times, most recently
from
August 24, 2026 05:53
66453ec to
0669d9d
Compare
another-rex
reviewed
Aug 25, 2026
another-rex
left a comment
Contributor
There was a problem hiding this comment.
Some initial comments
| <turbo-frame id="vulnerability-table-page-{{ .NextAfter }}" data-turbo-action="advance" target="_top" class="next-page-frame"> | ||
| <div class="next-page-container"> | ||
| <a class="next-page-button link-button" data-turbo-frame="_self" href="/list?page={{ add .Page 1 }}{{ if $.Query }}&q={{ $.EncodedQuery }}{{ end }}{{ if $.SelectedEcosystem }}&ecosystem={{ $.EncodedEcosystem }}{{ end }}"> | ||
| <a class="next-page-button link-button" data-turbo-frame="_self" href="/list?after={{ .NextAfter }}{{ if $.Query }}&q={{ $.Query }}{{ end }}{{ if $.SelectedEcosystem }}&ecosystem={{ $.SelectedEcosystem }}{{ end }}"> |
Contributor
There was a problem hiding this comment.
Is it fine to not encode the query anymore? Why was this changed?
Contributor
There was a problem hiding this comment.
Please add a comment if this is intentional.
Member
Author
There was a problem hiding this comment.
html/template actually automatically encodes these, and we don't want to double-encode them.
| } | ||
|
|
||
| // 3. Cold start fallback: fetch synchronously | ||
| return s.refreshEcosystemCounts(ctx) |
Contributor
There was a problem hiding this comment.
This feels like a good place for singleflight, so we don't have a bunch of refreshes all running at the start of the server.
Member
Author
There was a problem hiding this comment.
Made the singleflight change in #5840 (to avoid merge conflicts)
another-rex
approved these changes
Aug 25, 2026
michaelkedar
disabled the stack merge
August 25, 2026 03:00
michaelkedar
disabled the stack merge
August 25, 2026 03:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Connect the list/search page to Datastore.
after=timestamp_IDinstead of a page number - page numbers require re-iterating through all previous results in the database to arrive at the next page, which becomes very inefficient later pages. Swapping to a timestamp/id lets us use the database more correctly.page=Nfunctionality (with a warning log) so we can see if people have been manually scraping our websiteStack created with GitHub Stacks CLI • Give Feedback 💬