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

Show one generic message in case HPPS functionality is disabled #7372

Merged
merged 3 commits into from
Dec 13, 2023

Conversation

merkushin
Copy link
Member

Resolves #7362

Proposed Changes

  • Show a generic message when HPPS is disabled.

Testing Instructions

  1. Add 'set_time_limit' to disable_functions in your php.ini (Like that: disable_functions = set_time_limit, get_resources)
  2. Enable experimental UI feature flag: add_filter( 'sensei_feature_flag_experimental_features_ui', '__return_true' );
  3. Go to Sensei LMS > Settings > Experimental Features.
  4. Make sure you see a message like in the screenshot below.

CleanShot 2023-12-09 at 10 44 34@2x

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • New UIs match the designs
  • Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • Code is tested on the minimum supported PHP and WordPress versions
  • User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • "Needs Documentation" label is added if this change requires updates to documentation
  • Known issues are created as new GitHub issues

@merkushin merkushin added this to the 4.19.3 milestone Dec 9, 2023
@merkushin merkushin requested a review from a team December 9, 2023 16:54
@merkushin merkushin self-assigned this Dec 9, 2023
@merkushin
Copy link
Member Author

I suppose the failing checks are related to this PR: #7369

@donnapep
Copy link
Collaborator

Could we simplify this:

This feature is currently in development and should be used with caution. While HPPS is in its early experimental stage, the functionalities may be temporarily unavailable.

to something like this:

As this feature is currently experimental, it may not be available yet on some sites.

I think being in development and using "caution" are already implied by "experimental".

@merkushin
Copy link
Member Author

@donnapep

Here is the updated message:

CleanShot 2023-12-12 at 23 50 46@2x

The second line ("As this feature is currently experimental, it may not be available yet on some sites.") appears only when the feature is disabled.

Copy link

codecov bot commented Dec 13, 2023

Codecov Report

Merging #7372 (c911025) into trunk (8f61415) will increase coverage by 0.02%.
Report is 20 commits behind head on trunk.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk    #7372      +/-   ##
============================================
+ Coverage     50.74%   50.77%   +0.02%     
+ Complexity    11148    11145       -3     
============================================
  Files           613      613              
  Lines         47060    47042      -18     
  Branches        404      404              
============================================
+ Hits          23881    23884       +3     
+ Misses        22852    22831      -21     
  Partials        327      327              
Files Coverage Δ
includes/class-sensei-settings.php 57.85% <0.00%> (+0.88%) ⬆️

... and 4 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b49171...c911025. Read the comment docs.

Copy link
Collaborator

@donnapep donnapep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Message looks good to me!

@merkushin merkushin merged commit 844675b into trunk Dec 13, 2023
22 of 23 checks passed
@merkushin merkushin deleted the add/hpps-disabled-generic-message branch December 13, 2023 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't enable the HPPS feature on Atomic
2 participants