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

Closes #727 Update admin bar upsell #754

Merged
merged 13 commits into from
Nov 10, 2023

Conversation

Tabrisrp
Copy link
Contributor

@Tabrisrp Tabrisrp commented Nov 6, 2023

Description

Update the admin bar upsell block when under 20% of quota

Fixes #727

Type of change

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

Is the solution different from the one proposed during the grooming?

No

Checklists

Generic development checklist

  • My code follows the style guidelines of this project, with adapted comments and without new warnings.
  • I have added unit and integration tests that prove my fix is effective or that my feature works.
  • The CI passes locally with my changes (including unit tests, integration tests, linter).

Test summary

  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I validated all Acceptance Criteria of the related issues. (If applicable, provide proof).
  • I validated all test plan the QA Review asked me to.

@Tabrisrp Tabrisrp self-assigned this Nov 6, 2023
@Tabrisrp Tabrisrp marked this pull request as ready for review November 7, 2023 14:25
@Tabrisrp Tabrisrp requested a review from a team November 7, 2023 14:25
@vmanthos vmanthos self-requested a review November 8, 2023 10:10
@vmanthos
Copy link
Contributor

vmanthos commented Nov 8, 2023

@Tabrisrp Can you please take care of the conflicts when you have some time? 🙏
These came after merging #752.

Also, the $is_monthly isn't set in:
https://github.com/wp-media/imagify-plugin/blob/7b51656d42f35dbdbb8b1a8bf2267508cfafc205/classes/User/User.php

and this results in displaying the yearly offer regardless of what the user is using.

@Tabrisrp
Copy link
Contributor Author

Tabrisrp commented Nov 8, 2023

@vmanthos It's updated

@Tabrisrp Tabrisrp added this to the 2.1.3 milestone Nov 8, 2023
@vmanthos
Copy link
Contributor

vmanthos commented Nov 9, 2023

@Tabrisrp Thank you for the PR.

I can see the toolbar upsell box even when the available quota is >20%:
image

Also, for the Infinite plan, the following is displayed:
image

These ☝️ are from a real user, i.e. I haven't mocked the response.

Can you please look into this? 🙏

@Tabrisrp
Copy link
Contributor Author

Tabrisrp commented Nov 9, 2023

You're right I forgot one condition in the view, it should be good now

Copy link
Contributor

@vmanthos vmanthos 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 for the fixes, @Tabrisrp!

This is working as expected. 👍

TestRail report: Will be added when ready.

@vmanthos vmanthos added this pull request to the merge queue Nov 10, 2023
Merged via the queue into develop with commit e5555a3 Nov 10, 2023
6 checks passed
@vmanthos vmanthos deleted the enhancement/727-update-adminbar-status branch November 10, 2023 07:44
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.

Update the account status notification in the toolbar menu
3 participants