Closes #6432: Bail out from beacon conditions

[Miraeld]

Description

Fixes #6432

This PR modifies the beacon script to add a check for existing image data related to the URL in the database. If such data exists or the status is failed, the script process should be stopped. Additionally, a screen-size check should be added to the script to bail out under certain conditions.

Documentation

User documentation

This change does not directly impact users as it is related to the test setup and the beacon script.

Technical documentation

The add_lcp_data and check_lcp_data methods in the Controller.php file are used to add and check LCP data respectively. The main function in the lcp-beacon.js file has been modified to make an AJAX call to check if there are any records for the current URL and to check the screen size. If the screen size is not within the acceptable range, the function bails out.

Type of change

• Enhancement (non-breaking change which improves an existing functionality).

New dependencies

N/A

Risks

N/A

Checklists

Feature validation

• I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
• I triggered all changed lines of code at least once without new errors/warnings/notices.
• I implemented built-in tests to cover the new/changed code.

Documentation

• I prepared the user documentation for the feature/enhancement and shared it in the PR or the GitHub issue.
• The user documentation covers new/changed entry points (endpoints, WP hooks, configuration files, ...).
• I prepared the technical documentation if needed, and shared it in the PR or the GitHub issue.

Code style

• I wrote self-explanatory code about what it does.
• I wrote comments to explain why it does it.
• I named variables and functions explicitly.
• I protected entry points against unexpected inputs.
• I did not introduce unnecessary complexity.
• I listed the introduced external dependencies explicitly on the PR.
• I validated the repo-specific guidelines from CONTRIBUTING.md.

---

Acceptance Criteria Reminder

Bail-out if data is already available:

• Make sure there is a log in the beacon script in case it bails out because data is already available.
• Clear cache and ATF/LCP data. Make sure cache feature and LCP/ATF feature are enabled.
• Visit a page from desktop.
• Check in the DB that the LCP/ATF data is available for this page.
• Visit the page once again from desktop. Check in the dev tool console that the script about bailing out for data already there is printed out.
• Visit the page from mobile. Check that there is no log about bailing out for data already there.
• Check in the DB that the mobile LCP/ATF data has been added.

Bail-out if not acceptable screen-size: (Use BrowserStack or Rocket-E2E)

• Make sure there is a log in the script if it bails out because of screen size.
• Clear cache and LCP/ATF data.
• Visit a page with a screen-size 392 x 1000. The script should bail out with the right log.
• Visit a page with a screen-size 1000 x 829. The script should bail out with the right log.
• Visit a page with a screen-size 1000 x 1081. The script should bail out with the right log.
• Visit a page with a screen-size 1921 x 1080. The script should bail out with the right log.
• Visit a page with a screen-size 1920 x 1080. The script should complete and data must be available in the DB.
• Clear LCP/ATF data.
• Visit a page with a screen-size 393 x 830. The script should complete and data must be available in the DB.
• Clear LCP/ATF data.
• Visit a page with a screen-size 1000 x 1000. The script should complete and data must be available in the DB.

Check the screen-size filters:

• Repeat the above test about screen-size after applying different upper and lower limits with a filter:
  • Just set the upper limit: repeat the test (adapting the emulated screen-size).
  • Just set the lower limit: repeat the test (adapting the emulated screen-size).
• Validate that the filter is protected against unexpected inputs (null, empty, text, etc...)

[Tabrisrp]

Is this ready for review?

[Miraeld]

@Tabrisrp, yes it is ready.
We keep it like this at the moment. The documentation related to these changes can be found here.

[Tabrisrp]

You have some conflicts to fix in the JS

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -0.01% (target: -0.10%)	❌ 30.30% (target: 50.00%)

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (2486a81)	36946	14155	38.31%
Head commit (d783638)	36964 (+18)	14160 (+5)	38.31% (-0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#6555)	33	10	30.30%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy will stop sending the deprecated coverage status from June 5th, 2024. Learn more}

[MathieuLamiot]

@Miraeld There are conflicts to be resolved. Then it should be merged ASAP to avoid further conflicts with upcoming changes on the beacon.

[Miraeld]

@Mai-Saad, there is no test plan available for this yet,
However, I've tested this PR.
The way I've tested => Open a page with different dimensions while checking the devconsole for results.

Please find screenshots of the result as follow:

Desktop

Case 1

Opening a page in desktop mode while not meeting the size resolution requirements, we bail out.

Case 2

Opening a page in desktop while meeting the size resolution, we process and get results.

Case 3

Opening a page in desktop while we already have saved data within the database.

Mobile

Case 1

Opening a page in mobile mode while not meeting the size resolution requirements, we bail out.

Case 2

Opening a page in mobile while meeting the size resolution, we process and get results.

Case 3

Opening a page in mobile while we already have saved data within the database.

Please review this results for now and if everything's okay, we can move it to QA Done.
Thank you

[MathieuLamiot]

@Miraeld Maybe we need to test the new filters and also that they are guarded against bad values? (null, [], string, etc.). If wrong value, then the default value should be applied.

[Miraeld]

True that, Here is the following tests:

Changing values of the filter

Settings new threshold:

// Change the width threshold for the LCP beacon.
add_filter( 'rocket_lcp_width_threshold', function( $width_threshold, $is_mobile, $url ) {
	return $is_mobile ? 500 : 2500;
}, 10, 3 );

// Change the height threshold for the LCP beacon.
add_filter( 'rocket_lcp_height_threshold', function( $height_threshold, $is_mobile, $url ) {
	return $is_mobile ? 900 : 1200;
}, 10, 3 );

Thresholds are working perfectly.

Passing null & [] to filters

// Change the width threshold for the LCP beacon.
add_filter( 'rocket_lcp_width_threshold', function( $width_threshold, $is_mobile, $url ) {
	return [];
}, 10, 3 );

// Change the height threshold for the LCP beacon.
add_filter( 'rocket_lcp_height_threshold', function( $height_threshold, $is_mobile, $url ) {
	return null;
}, 10, 3 );

Result:

As values from the filter were not valid, we set back the default value.

Also please see #6593 for new threshold values modification

[MathieuLamiot]

Ok on my side 👍