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

Implemented server-side datatables for notes list for #3790 #3792

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

sylvieed
Copy link
Collaborator

Resolves #3790

After implementing DataTables on the notes & deeds list, these pages have performance issues on long lists because all of the rows must be loaded by the server at once.
To fix this, this PR changes the notes DataTable to use server-side processing. This means that the server only loads the currently displayed rows at once, eliminating the issue for large tables. This also means that all sorting and searching is handled on the server-side instead of the client-side, resulting in a slower UI (but it's consistent over all table lengths!)

I used the ajax-datatables-rails gem, which handles a majority of the server-side processing.

I split the deeds list and notes list, so the deeds list is always paginated and the notes list is always a DataTable. Now, the notes list has individual sortable columns rather than one large "deed" column.

The notes list for a collection:
image

The all notes list:
image

@saracarl
Copy link
Collaborator

Yay! This is great, and I think the fields for the Notes view is going to be useful.

Does this: so the deeds list is always paginated mean you can't see all deeds at once? (And if so is that what you and Ben decided on?)

@saracarl
Copy link
Collaborator

Test failure is this:

  1) Deed.order_by_recent_activity lists deeds in order by most recently created
     Failure/Error: expect(Deed.order_by_recent_activity.first).to eq(second_deed)
     
       expected: #<Deed id: 110, deed_type: "page_trans", page_id: nil, work_id: nil, collection_id: nil, article_id: ...ed_at: "2023-09-19 20:21:28", visit_id: nil, prerender: nil, prerender_mailer: nil, is_public: true>
            got: #<Deed id: 109, deed_type: "page_trans", page_id: nil, work_id: nil, collection_id: nil, article_id: ...ed_at: "2023-09-19 20:21:28", visit_id: nil, prerender: nil, prerender_mailer: nil, is_public: true>
     
       (compared using ==)
     
       Diff:
       @@ -1,5 +1,5 @@
       -#<Deed:0x0000000013204190
       - id: 110,
       +#<Deed:0x0000000013094c88
       + id: 109,
         deed_type: "page_trans",
         page_id: nil,
         work_id: nil,
       
     # ./spec/models/deed_spec.rb:27:in `block (3 levels) in <top (required)>'

@sylvieed
Copy link
Collaborator Author

Yes, you can't see all deeds at once. This just returns the deeds list to how it originally was -- it was never the plan to edit this list, I just did because it was the same view as the notes list.

deed list

@sylvieed
Copy link
Collaborator Author

@benwbrum this is ready to merge.

@benwbrum benwbrum merged commit 95f1934 into development Sep 20, 2023
1 check passed
@benwbrum benwbrum deleted the 3790-server-side-datatables branch September 20, 2023 18:02
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.

deed#notes takes two hours to render
3 participants