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

Add red bubble with update count on in-app My Subscriptions tab #46088

Merged
merged 13 commits into from Apr 2, 2024

Conversation

thilinah
Copy link
Contributor

@thilinah thilinah commented Apr 1, 2024

Submission Review Guidelines:

Changes proposed in this Pull Request:

  • This PR adds a red badge on My Subscription tab of the store if it isn't connected.

Closes # .

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Start the development environment pnpm -- wp-env start
  2. Build assets using pnpm --filter='@woocommerce/plugin-woocommerce' build
  3. Your store shouldn't be connected to Woo.com and the Woo Update Manager plugin shouldn't be installed.
  4. Visit WCCore My Subscriptions page, credentials can be found under wp-env documentation
  5. Notice the red bubble on in-app, My Subscriptions tab showing 1
Screenshot 2024-04-01 at 13 47 36
  1. Connect your site. After the site is connected, if the Woo.com Update Manager isn't installed the red bubble on Extensions menu and the My Subscriptions tab should show number of updates Available for Woo.com extensions + 1.
  2. For an example in the screenshot AW has an update available, so the update count is one, since Woo.com update manager isn't installed it should display 2.
Screenshot 2024-04-01 at 13 52 06
  1. If AutomateWoo is updated to the latest version, update count should only show 1 as Woo.com Update Manager isn't installed.
Screenshot 2024-04-01 at 15 29 31
  1. Install WUM by downloading it via : https://woo.com/product-download/woo-update-manager

  2. If WUM is installed but not activated, still the red bubble should be displayed.

Screenshot 2024-04-01 at 15 33 47
  1. When the WUM is activated and no other Woo.com extension are needed to be updated, the red bubble should not be displayed.
Screenshot 2024-04-01 at 15 35 42
  1. When the site is disconnected, the red badge should be shown regardless of WUM installtion status.

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Displays a red badge on in-app My Subscriptions tab if Woo.com Update Manager is not installed or activated

Comment

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Apr 1, 2024
@thilinah thilinah requested review from denho and anaspk April 1, 2024 13:41
Copy link
Contributor

github-actions bot commented Apr 1, 2024

Hi @laurendavissmith, @anaspk, @denho,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@@ -60,6 +62,7 @@ function ( $product ) {
'wooUpdateManagerActive' => WC_Woo_Update_Manager_Plugin::is_plugin_active(),
'wooUpdateManagerInstallUrl' => WC_Woo_Update_Manager_Plugin::generate_install_url(),
'wooUpdateManagerPluginSlug' => WC_Woo_Update_Manager_Plugin::WOO_UPDATE_MANAGER_SLUG,
'wooUpdateCount' => WC_Woo_Update_Manager_Plugin::increment_update_count_for_woo_update_manager( WC_Helper_Updater::get_updates_count() ?? 0 ),
Copy link
Member

Choose a reason for hiding this comment

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

The logic of getting the count here duplicates the logic in get_updates_count_html. Would it be possible to standardize and have just one function that returns the final count?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed here: f09891b

Copy link
Contributor

@anaspk anaspk left a comment

Choose a reason for hiding this comment

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

Thank you @thilinah , LGTM and works as described. Just posted a minor comment.

Copy link
Member

@denho denho left a comment

Choose a reason for hiding this comment

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

LGTM!

$count_html = sprintf( '<span class="update-plugins count-%d"><span class="update-count">%d</span></span>', $count, number_format_i18n( $count ) );

Copy link
Member

Choose a reason for hiding this comment

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

The bubble HTML is still present in the DOM when the number is count is 0. It's not seen, as there is CSS that hides the element. Guess it's fine to leave as is, but just wanted to bring it to your attention in case it's not intended.

image

@nigeljamesstevenson nigeljamesstevenson merged commit 561a3d1 into trunk Apr 2, 2024
27 checks passed
@nigeljamesstevenson nigeljamesstevenson deleted the update/red-bubble-on-my-subs-tab branch April 2, 2024 15:04
@github-actions github-actions bot added this to the 8.9.0 milestone Apr 2, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Apr 2, 2024
@Stojdza Stojdza modified the milestones: 8.9.0, 8.8.0 Apr 2, 2024
github-actions bot pushed a commit that referenced this pull request Apr 2, 2024
* Extract update count function.

* Pass wooUpdateCount to be used by admin components.

* Show update count bubble on in-app my subscriptions tab.

* Add changefile(s) from automation for the following project(s): woocommerce

* Refactor how update count HTML is generated and move logic into appropriate classes.

* Show red badge when the site is disconnected, regardless of the installation status of WUM.

* Always display as 1 update is available when site is disconnected.

* Skip using dangerouslySetInnerHTML.

* Remove unused dependency.

* Move updateCount to a tab property.

* Prevent calling get_update_count if the site is not connected.

* Raw url encode the product key before signing the URL

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Muhammad Anas <muhammad.anas@automattic.com>
Stojdza pushed a commit that referenced this pull request Apr 2, 2024
* Add red bubble with update count  on in-app My Subscriptions tab (#46088)

* Extract update count function.

* Pass wooUpdateCount to be used by admin components.

* Show update count bubble on in-app my subscriptions tab.

* Add changefile(s) from automation for the following project(s): woocommerce

* Refactor how update count HTML is generated and move logic into appropriate classes.

* Show red badge when the site is disconnected, regardless of the installation status of WUM.

* Always display as 1 update is available when site is disconnected.

* Skip using dangerouslySetInnerHTML.

* Remove unused dependency.

* Move updateCount to a tab property.

* Prevent calling get_update_count if the site is not connected.

* Raw url encode the product key before signing the URL

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Muhammad Anas <muhammad.anas@automattic.com>

* Prep for cherry pick 46088

---------

Co-authored-by: Thilina Pituwala <thilina.hasantha@gmail.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Muhammad Anas <muhammad.anas@automattic.com>
Co-authored-by: WooCommerce Bot <no-reply@woo.com>
@rodelgc rodelgc added needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Apr 2, 2024
senadir pushed a commit that referenced this pull request Apr 4, 2024
* Extract update count function.

* Pass wooUpdateCount to be used by admin components.

* Show update count bubble on in-app my subscriptions tab.

* Add changefile(s) from automation for the following project(s): woocommerce

* Refactor how update count HTML is generated and move logic into appropriate classes.

* Show red badge when the site is disconnected, regardless of the installation status of WUM.

* Always display as 1 update is available when site is disconnected.

* Skip using dangerouslySetInnerHTML.

* Remove unused dependency.

* Move updateCount to a tab property.

* Prevent calling get_update_count if the site is not connected.

* Raw url encode the product key before signing the URL

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Muhammad Anas <muhammad.anas@automattic.com>
senadir pushed a commit that referenced this pull request Apr 4, 2024
* Extract update count function.

* Pass wooUpdateCount to be used by admin components.

* Show update count bubble on in-app my subscriptions tab.

* Add changefile(s) from automation for the following project(s): woocommerce

* Refactor how update count HTML is generated and move logic into appropriate classes.

* Show red badge when the site is disconnected, regardless of the installation status of WUM.

* Always display as 1 update is available when site is disconnected.

* Skip using dangerouslySetInnerHTML.

* Remove unused dependency.

* Move updateCount to a tab property.

* Prevent calling get_update_count if the site is not connected.

* Raw url encode the product key before signing the URL

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Muhammad Anas <muhammad.anas@automattic.com>
@thilinah thilinah self-assigned this Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants