New Recents + Popular API Format - Frontend #42305
Merged
Conversation
This file contains 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
Closed
iethree
force-pushed
the
beefup-recents-frontend
branch
from
934e2ea
to
277fff3
Compare
iethree
changed the title
iethree
changed the title
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 8bb8b77...2d23023.
|
iethree
force-pushed
the
beefup-recents-frontend
branch
from
277fff3
to
e032be7
Compare
|
rafpaf
reviewed
May 8, 2024
| card: Card | null, | ||
| card: Partial< | ||
| Pick<Card, "id" | "name" | "type" | "card_id" | "model"> | ||
| > | null, |
There was a problem hiding this comment.
I might be off my head but would Partial<Card> | null work here?
There was a problem hiding this comment.
It would work, but I kind of like how Pick describes what the component actually might use. WDYT?
rafpaf
approved these changes
May 8, 2024
This was referenced May 9, 2024
iethree
force-pushed
the
beefup-recents-frontend
branch
2 times, most recently
from
4a26947
to
ecbdc46
Compare
iethree
force-pushed
the
beefup-recents-frontend
branch
from
3bea769
to
2d23023
Compare
escherize
force-pushed
the
bcm-beefup-recents
branch
from
8bb8b77
to
ccfc00b
Compare
iethree
added a commit
that referenced
this pull request
May 13, 2024
* fix tests * new API payload format for recents and populars * update url generation * update recents components * update tests * only show 5 recents in command palette * remove unused types * update tests * obey the linter * type updates * fix unit test * change where we filter recents --------- Co-authored-by: Bryan Maass <[email protected]>
escherize
added a commit
that referenced
this pull request
May 13, 2024
* wip * adds more collection-touch events * remove sneaky dep that slipped in * cleaning up, and adding tests * respond to review comments + mini bug reports * make it official * double official * cleanup + return recent-views/Item for `GET activity/popular_items` * get popular_items returning the proper shape * compare doesn't what I thought * add display_name to tables * table.database.name should be the db name, not the table name * add table.database.initial_sync_status * improve call to toucan2 * light renaming + many text fixes * include moderated_status in cards and datasets * look up Dashboard permissions for Dashboards * stop double-querying for `can-write?` checks - also include some debug logging * fix card <-> model dispatching also gets tests passing locally * let the test runner run the tests * fix gc test * adds description * description can be null * better tests no more nils passed through the app * debugging test * fix tests and skip archived * mysql finesse * ellide inactive tables * wip * adds more collection-touch events * remove sneaky dep that slipped in * cleaning up, and adding tests * respond to review comments + mini bug reports * make it official * double official * cleanup + return recent-views/Item for `GET activity/popular_items` * get popular_items returning the proper shape * compare doesn't what I thought * add display_name to tables * table.database.name should be the db name, not the table name * add table.database.initial_sync_status * improve call to toucan2 * light renaming + many text fixes * include moderated_status in cards and datasets * look up Dashboard permissions for Dashboards * stop double-querying for `can-write?` checks - also include some debug logging * fix card <-> model dispatching also gets tests passing locally * let the test runner run the tests * fix gc test * adds description * description can be null * better tests no more nils passed through the app * debugging test * fix tests and skip archived * mysql finesse * New Recents + Popular API Format - Frontend (#42305) * fix tests * new API payload format for recents and populars * update url generation * update recents components * update tests * only show 5 recents in command palette * remove unused types * update tests * obey the linter * type updates * fix unit test * change where we filter recents --------- Co-authored-by: Bryan Maass <[email protected]> * fix modelToUrl types * respond to review comments * post merge test fix --------- Co-authored-by: Ryan Laurie <[email protected]> Co-authored-by: Ryan Laurie <[email protected]>
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.
Note
The frontend code here is ready for review, but there is one little piece missing in the backend: models should return with
model: dataset, but currently they havemodel: cardImportant
I highly recommend looking at this PR by commit, rather than all at once. The first few commits have all the substance, and the bulk of the PR is just cleaning up types and test mocks
Closes #41957
Description
Uses a better, beefier recents format from the backend, and handles a larger number of recent items.
Recents appear in:
Popular items only appear on the home page for brand new users.
To test this component, i'd recommend just modifying the if statement in
HomeContent.tsxlike so:Checklist