Load full status report on demand with Ajax #4095
Open
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
oscarssanchezz
changed the title
felipeelia
requested changes
Feb 26, 2025
There was a problem hiding this comment.
In addition to my comments, @oscarssanchezz:
- This needs both unit and e2e tests;
- It seems the "Indexable Content" information is not present if you download or copy the report
- Did you explore breaking ajax reports into separate requests? Right now, we only have one report that relies on ajax, but if we need another in the future, wouldn't it be better if we could fire multiple (parallel) ajax requests instead of a giant single request?
- I'm a bit concerned with the UX here. Shouldn't we allow people to download a partial report?
|
|
||
| /** | ||
| * Internal dependencies. | ||
| */ | ||
| import Value from './report/value'; | ||
| // import ReportHeader from './report/header'; |
There was a problem hiding this comment.
I guess this was a left-over and can be removed?
| } | ||
|
|
||
| if ( empty( $this->formatted_reports[ $post['report'] ] ) ) { | ||
| wp_send_json_error( [ 'message' => 'Report not found.' ], 404 ); |
There was a problem hiding this comment.
This string needs to be translatable, no?
| 'groups' => ! $report->is_ajax_report() ? $report->get_groups() : [], | ||
| 'messages' => $report->get_messages(), | ||
| 'title' => $report->get_title(), | ||
| 'is_ajax_report' => $report->is_ajax_report(), |
There was a problem hiding this comment.
for variables we are passing to JS, we are using camelCase, so this will need to be isAjaxReport.
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.
Description of the Change
This PR adds functionality to load resource intensive reports on demand with a "Generate Full Status Report" button.
If the full status report has not been generated yet, it should show a warning message declared by the specific report, prompting users to generate the full status report to see it.
If the status report has not been generated yet, copy to clipboard and download status report buttons should not be available, as these will contain an incomplete status report.
epreport.mov
Closes #3767
How to test the Change
1.- Open Status Report page.
2.- Check the status reports that should be loaded with ajax are showing the correct notice (only indexables at this point).
3.- Download status report and copy to clipboard buttons should be disabled.
4.- Click on Generate Full status report.
5.- Report should now show all groups and notice should be gone.
6.- Download status report and copy to clipboard should have full Info.
7.- Generate Full status report button should be disabled.
Changelog Entry
Credits
Props @oscarssanchez , @felipeelia
Checklist: