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

Update Stellar Sale Banner and Logic #2095

Merged
merged 13 commits into from
Jun 5, 2024

Conversation

codingmusician
Copy link
Contributor

@codingmusician codingmusician commented Jun 4, 2024

🎫 Ticket

ET-2113

πŸ—’οΈ Description

Updated the banner for the 2024 Stellar Sale.

πŸŽ₯ Artifacts

image

βœ”οΈ Checklist

  • Changelog entry in the readme.txt file.
  • Code is covered by NEW wpunit or integration tests.
  • Code is covered by EXISTING wpunit or integration tests.
  • Are all the required tests passing?
  • Automated code review comments are addressed.
  • Have you added Artifacts?
  • Check the base branch for your PR.
  • Add your PR to the project board for the release.

@codingmusician codingmusician self-assigned this Jun 4, 2024
src/Tribe/Admin/Notice/Marketing/Stellar_Sale.php Outdated Show resolved Hide resolved
src/Tribe/Admin/Notice/Marketing/Stellar_Sale.php Outdated Show resolved Hide resolved
src/Tribe/Admin/Notice/Marketing/Stellar_Sale.php Outdated Show resolved Hide resolved
src/Tribe/Admin/Notice/Marketing/Stellar_Sale.php Outdated Show resolved Hide resolved
src/Tribe/Admin/Notice/Marketing/Stellar_Sale.php Outdated Show resolved Hide resolved
src/admin-views/notices/tribe-stellar-sale.php Outdated Show resolved Hide resolved
src/admin-views/notices/tribe-stellar-sale.php Outdated Show resolved Hide resolved
src/admin-views/notices/tribe-stellar-sale.php Outdated Show resolved Hide resolved
src/admin-views/notices/tribe-stellar-sale.php Outdated Show resolved Hide resolved
src/admin-views/notices/tribe-stellar-sale.php Outdated Show resolved Hide resolved
src/Tribe/Admin/Notice/Marketing/Stellar_Sale.php Outdated Show resolved Hide resolved
src/Tribe/Admin/Notice/Marketing/Stellar_Sale.php Outdated Show resolved Hide resolved
src/Tribe/Admin/Notice/Marketing/Stellar_Sale.php Outdated Show resolved Hide resolved
src/admin-views/notices/tribe-stellar-sale.php Outdated Show resolved Hide resolved
src/admin-views/notices/tribe-stellar-sale.php Outdated Show resolved Hide resolved
tests/wpunit/Tribe/Notices/Stellar_SaleTest.php Outdated Show resolved Hide resolved
tests/wpunit/Tribe/Notices/Stellar_SaleTest.php Outdated Show resolved Hide resolved
tests/wpunit/Tribe/Notices/Stellar_SaleTest.php Outdated Show resolved Hide resolved
tests/wpunit/Tribe/Notices/Stellar_SaleTest.php Outdated Show resolved Hide resolved
tests/wpunit/Tribe/Notices/Stellar_SaleTest.php Outdated Show resolved Hide resolved
tests/wpunit/Tribe/Notices/Stellar_SaleTest.php Outdated Show resolved Hide resolved
@codingmusician codingmusician added the code review Status: requires a code review. label Jun 5, 2024
@codingmusician codingmusician marked this pull request as ready for review June 5, 2024 18:52
Copy link
Member

@dpanta94 dpanta94 left a comment

Choose a reason for hiding this comment

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

Looks good! Approving with optional suggestion

Comment on lines +70 to 85
// Determine the copy based on the active plugins.
$has_events_calendar_pro = $dependency->is_plugin_active( 'Tribe__Events__Pro__Main' );
$has_event_tickets_plus = $dependency->is_plugin_active( 'Tribe__Tickets_Plus__Main' );

if ( $has_events_calendar_pro && ! $has_event_tickets_plus ) {
$template_args['sub_heading'] = __( 'Save 40% on Filter Bar.', 'tribe-common' );
$template_args['stellar_copy'] = __( 'Add filters to your calendar during the annual Stellar Sale. Now through July 30.', 'tribe-common' );
} elseif ( $has_event_tickets_plus && ! $has_events_calendar_pro ) {
$template_args['sub_heading'] = __( 'Save 40% on The Events Calendar Bundles.', 'tribe-common' );
$template_args['stellar_copy'] = __( 'Take <b>40%</b> off when you upgrade to a bundle during the annual Stellar Sale. Now through July 30.', 'tribe-common' );
} else {
$template_args['heading'] = __( 'Make it stellar.', 'tribe-common' );
$template_args['sub_heading'] = __( 'Save 40% on all StellarWP products.', 'tribe-common' );
$template_args['stellar_copy'] = __( 'Take <b>40%</b> off all brands during the annual Stellar Sale. Now through July 30.', 'tribe-common' );
}
}
Copy link
Member

@dpanta94 dpanta94 Jun 5, 2024

Choose a reason for hiding this comment

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

Completely optional and up to you. - Could we refactor this whole if - elseif - else to early return instead ?

e.g.

if ( ! $dependency->has_active_premium_plugin() ) {
    return $this->get_template()->template( 'notices/tribe-stellar-sale', $template_args, false );
}

if ( $has_events_calendar_pro && $has_event_tickets_plus ) {
    // $template_args[]...
    return;
}

if ( ! $has_events_calendar_pro ) {
return;
}

// has only ECP
return;



Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't want to return, because we are passing the template args into the template on line 87.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would be returning the template in every scenario, in that case, repeating the same line of code over and over.

@codingmusician codingmusician merged commit 6d0ea85 into release/T24.asag Jun 5, 2024
11 of 12 checks passed
@codingmusician codingmusician deleted the tweak/stellar-sale-banner branch June 5, 2024 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code review Status: requires a code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants