-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Chore(Inertia): Migrate Collabs page APIs to Inertia partial reloads #2811
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
base: main
Are you sure you want to change the base?
Conversation
| def memberships_paged | ||
| props = CollabProductsPagePresenter.new(**presenter_params).memberships_table_props | ||
|
|
||
| render json: { | ||
| pagination: props[:memberships_pagination], | ||
| entries: props[:memberships], | ||
| } | ||
| end | ||
|
|
||
| def products_paged | ||
| props = CollabProductsPagePresenter.new(**presenter_params).products_table_props | ||
|
|
||
| render json: { | ||
| pagination: props[:products_pagination], | ||
| entries: props[:products], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed separate API endpoints as we're switched to Inertia Partial reload.
| render inertia: "Products/Collabs/Index", props: { | ||
| stats: -> { presenter.initial_page_props[:stats] }, | ||
| archived_tab_visible: -> { presenter.initial_page_props[:archived_tab_visible] }, | ||
| collaborators_disabled_reason: -> { presenter.initial_page_props[:collaborators_disabled_reason] }, | ||
| products: -> { presenter.products_table_props[:products] }, | ||
| products_pagination: -> { presenter.products_table_props[:products_pagination] }, | ||
| products_sort: -> { presenter.products_sort }, | ||
| memberships: -> { presenter.memberships_table_props[:memberships] }, | ||
| memberships_pagination: -> { presenter.memberships_table_props[:memberships_pagination] }, | ||
| memberships_sort: -> { presenter.memberships_sort }, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switched to inertia partial reload, used lazy evaluated props for partial reload.
| import { getPagedMemberships, MembershipsParams, Membership } from "$app/data/collabs"; | ||
| import { Membership } from "$app/data/collabs"; | ||
| import { classNames } from "$app/utils/classNames"; | ||
| import { formatPriceCentsWithCurrencySymbol } from "$app/utils/currency"; | ||
| import { asyncVoid } from "$app/utils/promise"; | ||
| import { AbortError, assertResponseError } from "$app/utils/request"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed client-side fetching related imports
| const onSetSort = (newSort: Sort<MembershipSortKey> | null) => { | ||
| setSort(newSort); | ||
| setIsLoading(true); | ||
| router.reload({ | ||
| data: { | ||
| memberships_sort_key: newSort?.key, | ||
| memberships_sort_direction: newSort?.direction, | ||
| memberships_page: undefined, | ||
| }, | ||
| only: ["memberships", "memberships_pagination", "memberships_sort"], | ||
| onFinish: () => setIsLoading(false), | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced custom loadMemberships/loadProducts fetch functions with Inertia's router.reload()
| <NavigationButton | ||
| <NavigationButtonInertia |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
collaborators/new page is now migrated to inertia, so to avoid full reload we'll use inertia based NavigationButtonInertia.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Removed getPagedProducts and getPagedMemberships functions ( no longer needed, as we'll use inertia partial reload )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed products_paged and memberships_paged routes, these endpoints are no longer needed with Inertia partial reloads.
|
|
||
| [membership_collab_1, membership_collab_2].each do |product| | ||
| expect(memberships.any? { |m| m[:id] == product.id }).to be(true) | ||
| expect(memberships.any? { |m| m["id"] == product.id }).to be(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed symbol keys to string keys (m[:id] → m["id"]), as Inertia props come back as string keys
| end | ||
|
|
||
| describe "GET memberships_paged" do | ||
| describe "GET index with pagination" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated pagination/sorting tests, now test via index action params instead of separate endpoints.
|
|
||
| describe "GET products_paged" do | ||
| before { stub_const("CollabProductsPagePresenter::PER_PAGE", 1) } | ||
| describe "GET index with sorting" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added sorting tests, verifies products_sort and memberships_sort params work correctly.
| PER_PAGE = 10 | ||
|
|
||
| def initialize(pundit_user:, page: 1, sort_params: {}, query: nil) | ||
| attr_reader :products_sort, :memberships_sort |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are exposed publicly so the controller can pass them back to the frontend as props
(products_sort: -> { presenter.products_sort })| <CollabsMembershipsTable | ||
| entries={memberships} | ||
| pagination={membershipsPagination} | ||
| sort={membershipsSort} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added sort props, passes memberships_sort and products_sort to table components.
|
|
||
| private | ||
| attr_reader :pundit_user, :page, :sort_params, :query, :total_revenue, :total_customers, :total_members, :total_collaborations | ||
| attr_reader :pundit_user, :products_page, :memberships_page, :query, :total_revenue, :total_customers, :total_members, :total_collaborations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously there was a single page param shared by both tables ( each table were using separate endpoints to make requests). now we use single index endpoint with partial reload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed unused routes, as we'll use single index endpoint with partial reload to fetch data.
| import { getPagedProducts, ProductsParams, Product } from "$app/data/collabs"; | ||
| import { Product } from "$app/data/collabs"; | ||
| import { classNames } from "$app/utils/classNames"; | ||
| import { formatPriceCentsWithCurrencySymbol } from "$app/utils/currency"; | ||
| import { asyncVoid } from "$app/utils/promise"; | ||
| import { AbortError, assertResponseError } from "$app/utils/request"; | ||
|
|
||
| import { Pagination, PaginationProps } from "$app/components/Pagination"; | ||
| import { ProductIconCell } from "$app/components/ProductsPage/ProductIconCell"; | ||
| import { showAlert } from "$app/components/server-components/Alert"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed client-side fetching related imports, as we'll use inertia partial reload
|
|
||
| const { entries: products, pagination, isLoading } = state; | ||
|
|
||
| const loadProducts = asyncVoid(async ({ page, query }: ProductsParams) => { | ||
| setState((prevState) => ({ ...prevState, isLoading: true })); | ||
| try { | ||
| activeRequest.current?.cancel(); | ||
|
|
||
| const request = getPagedProducts({ | ||
| page, | ||
| query, | ||
| }); | ||
| activeRequest.current = request; | ||
|
|
||
| setState({ | ||
| ...(await request.response), | ||
| isLoading: false, | ||
| query, | ||
| }); | ||
| activeRequest.current = null; | ||
| tableRef.current?.scrollIntoView({ behavior: "smooth" }); | ||
| } catch (e) { | ||
| if (e instanceof AbortError) return; | ||
| assertResponseError(e); | ||
| setState((prevState) => ({ ...prevState, isLoading: false })); | ||
| showAlert(e.message, "error"); | ||
| } | ||
| }); | ||
|
|
||
| const { items, thProps } = useClientSortingTableDriver<Product>(state.entries); | ||
| const [sort, setSort] = React.useState<Sort<ProductSortKey> | null>(props.sort); | ||
| const items = props.entries; | ||
|
|
||
| const onSetSort = (newSort: Sort<ProductSortKey> | null) => { | ||
| setSort(newSort); | ||
| setIsLoading(true); | ||
| router.reload({ | ||
| data: { | ||
| products_sort_key: newSort?.key, | ||
| products_sort_direction: newSort?.direction, | ||
| products_page: undefined, | ||
| }, | ||
| only: ["products", "products_pagination", "products_sort"], | ||
| onFinish: () => setIsLoading(false), | ||
| }); | ||
| }; | ||
|
|
||
| const thProps = useSortingTableDriver<ProductSortKey>(sort, onSetSort); | ||
|
|
||
| const handlePageChange = (page: number) => { | ||
| setIsLoading(true); | ||
| router.reload({ | ||
| data: { products_page: page }, | ||
| only: ["products", "products_pagination", "products_sort"], | ||
| onFinish: () => { | ||
| setIsLoading(false); | ||
| tableRef.current?.scrollIntoView({ behavior: "smooth" }); | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to use Inertia partial reload, rather than client side API based fetching
|
@binary-koan could you please review this PR? Thanks! |
Issue: #856
Description
Before/After
Before:
Screen.Recording.2026-01-10.at.3.43.59.PM.mov
After:
Screen.Recording.2026-01-10.at.2.25.04.PM.mov
Test Results
Checklist
AI Disclosure
used claude-code with opus 4.5 to assist with code change