Feature 59 ajax for project table#108
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements server-side AJAX data loading for the project table, removing the dependency on cached project data and standardizing date formatting across the application.
Key Changes:
- Migrated project table from client-side rendering with cached data to server-side AJAX pagination
- Introduced
format_datehelper function to standardize date formatting across detail and table views - Added data normalization and coercion functions (
normalize_project_data,coerce_project_page) to handle API responses
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/testthat/test_project_table.R |
Updated tests to validate AJAX configuration and data processing pipeline instead of cached data handling |
R/table_project.R |
Complete refactor to implement remote data loading with normalization, coercion, and display transformation functions |
R/table.R |
Added clean_column_dates helper function for consistent date formatting in table columns |
R/server.R |
Removed cached project data loading logic and updated table render call to use parameterless AJAX approach |
R/detail_reference.R |
Refactored to use new format_date helper for date formatting consistency |
R/detail_project.R |
Standardized date field formatting using format_date helper across all date columns |
R/detail_helpers.R |
Added new format_date helper that wraps safe_parse_date and handles formatting with default fallback |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
regetz
left a comment
There was a problem hiding this comment.
Nice, one more table done, thanks! I added a few comments, but nothing big, and two of them are about future to-dos anyway.
| project_description = shiny::renderUI({ | ||
| htmltools::tags$div(id = "project-description", htmltools::HTML(result$project_description)) | ||
| }), | ||
| # TODO: Implement contributor listing when API supports cross-resource queries |
There was a problem hiding this comment.
Or, just a thought, we could add a relevant nested field to the data returned by the parties endpoint. I mean, I kinda prefer to keep the API simpler and have clients just make the cross-resource request as needed, but we did already establish a precedent for the nested field. 🤷
There was a problem hiding this comment.
@regetz I was actually gonna ask you if that was possible. I think it's the more parsimonious solution since we have the precedent of nested fields already.
| parsed | ||
| } | ||
|
|
||
| # TODO: Eventually should add a link to see details with full text at the end of truncated text |
There was a problem hiding this comment.
👍 Yeah I was thinking this too
| ) | ||
| ) | ||
| descriptions <- clean_column_data(project_data, "project_description") | ||
| descriptions <- truncate_text_with_ellipsis(descriptions, max_chars = 680L) |
There was a problem hiding this comment.
Any reason why 680 characters, and not something shorter? Long descriptions make for some rather tall rows, and it's easy enough for users to click on the Details button to see the full description. Not a show-stopper to leave this as-is for now from my POV, but just throwing it out there in case you were thinking in the same direction.
There was a problem hiding this comment.
680 just happened to line up with the max height of a row to fit in the height of my screen. Pretty arbitrary and I'm definitely down to go smaller if that's your preference.
What:
Closes #59 by implementing dataTablesAjax server-side search and paging functionality to the project table. Also standardizes some date formatting for the detail and table views and truncates the description column text in the table.
Why:
To further minimize the data stored in the shiny server session and align with the approach taken for concept tables.
How:
Docs and Testing:
Updated project table tests to enforce new configuration and test processing function normalizes rows
All tests pass and devtools::check() runs with only a warning about using unexported get_all_resources from vegbankr