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

Enhancement/317-169 Ability to set product-level check-in/out times #328

Merged
merged 14 commits into from
May 12, 2023

Conversation

faisal-alvi
Copy link
Member

@faisal-alvi faisal-alvi commented Feb 3, 2023

All Submissions:

  • Does your code follow the WordPress coding standards?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changes proposed in this Pull Request:

This PR introduces a new filter woocommerce_accommodation_booking_get_check_times that allows users to filter the check-in/out times per product.

Closes #169,
Closes #317.

How to test the changes in this Pull Request:

  1. Visit WP Admin → Bookings → Settings → Accommodation (tab).
  2. Change the check-in time to 9 AM and check-out to 9 AM. (this is just an example)
  3. Open any accommodation product and note that when you add it in the cart, it displays the 9 AM as check-in and check-out times.
  4. Also, if you are in a storefront theme, you would see “Arriving/leaving” tab on the product page with the same times.
  5. Now add the following code in your theme's functions.php:
add_filter( 'woocommerce_accommodation_booking_get_check_times', function( $check_time, $type, $product_id ) {
	if ( 123 === (int) $product_id ) {
		$check_time = 'in' === $type ? '13:00' : $check_time;
		$check_time = 'out' === $type ? '14:00' : $check_time;
	}

	return $check_time;
}, 10, 3 );
  1. Make sure to change the product ID in above code from 123 to your accommodation product's ID.
  2. Now check again, the check-in time should be 1 PM and check-out time should be 2 PM (because we’ve filtered it using the above code).
  3. Try changing the timings in the code and confirm it works as expected.
  4. Also, test various places if the filtered timings display correctly. For example, on cart page, check out page, order success page, bookings edit page, etc.

Other information:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changelog entry

Dev - Added a new filter, woocommerce_accommodation_booking_get_check_times, to change the check-in/out timings per product.

@faisal-alvi faisal-alvi added this to the 1.1.38 milestone Feb 3, 2023
@faisal-alvi faisal-alvi self-assigned this Feb 3, 2023
@faisal-alvi faisal-alvi changed the title Enhancement/317 Ability to set product-level check-in/out times Enhancement/317-169 Ability to set product-level check-in/out times Feb 3, 2023
@dkotter dkotter added needs: regression The Issue/PR needs regresstion testing. needs: Woo review needs: UAT labels Feb 3, 2023
@ankitguptaindia
Copy link
Member

QA Report - Need Fixes ❌

@faisal-alvi Changed time is working expected on other places except the Booking Edit page. Booking edit page still shows time which is coming from Plugin Global Setting.

Plugin Global Setting-
Screenshot 2023-02-07 at 8 41 06 PM

Edit Booking Screen -
Screenshot 2023-02-07 at 8 42 04 PM

Order Edit Screen -

Screenshot 2023-02-07 at 8 44 54 PM

Customer's email Checked ✅
Order email ✅
Booking Calendar ✅
Cart ✅
Checkout ✅
Thank you page ✅
My Account Order Page ✅
My Account Bookings Page ✅

Next Step- Moving back to re-fix.

@faisal-alvi
Copy link
Member Author

faisal-alvi commented Mar 2, 2023

@ankitguptaindia, I have investigated the issue you reported and was able to reproduce it under the following conditions (and also fixed it):

  1. Check-in time and check-out time are both set to 10 AM in the settings.
  2. A booking is placed without using the filter.
  3. Then, a filter is added in the functions.php file with a check-in time of 1 PM and a check-out time of 2 PM.
  4. When viewing the booking edit screen, the check-in and check-out times are displayed correctly as 10 AM and 10 AM respectively, which is expected because the booking was made before the filter was applied.
  5. However, on the order edit screen, the check-in and check-out times in the highlighted box show as 1 PM and 2 PM, which is incorrect and has been fixed.
  6. The same issue is observed on the bookings listing page where the start time and end time columns show 1 PM and 2 PM, even though the booking was made for 10 AM and 10 AM. This has also been fixed.

Moving the linked issue tickets back to the QA column (Skipping 10up code re-review as the change looks fine).

@vikrampm1 vikrampm1 modified the milestones: 1.1.39, Future Release Mar 15, 2023
@ankitguptaindia
Copy link
Member

Hi @faisal-alvi
Currently, the product level check-in and check-out time are visible on the single product page. However, in other places such as My Account, Order Thank You Page, Edit Order Page, etc., the time is still being displayed from the Accommodation Plugin Core Setting.

Steps:

  1. Copy the above code snippet from Point-5 and paste in theme's function.php
  2. Change Product ID
  3. Open a single product page and verify - **Check-in and Check-out ** (appearing according to filter)
  4. Place a booking
  5. Now check **Check-in and Check-out ** time on Order Thank you page and Edit order page
  6. Check-in and Check-out times are appearing to the plugin setting.

image

image

Issue.with.Check-in.Check-out.time.mp4

Next Step- Moving back to re-fix.

@ankitguptaindia ankitguptaindia added the status: needs refresh The Issue/PR needs re-fix label Mar 29, 2023
@faisal-alvi
Copy link
Member Author

@ankitguptaindia thanks for the report. I tested again and the correct timings are displaying for me on product page, cart page, checkout page, order edit page, edit bookings page, my accounts > bookings page, etc.

image

Let's connect 1:1 to discuss this in detail and test together on a call.

@faisal-alvi
Copy link
Member Author

@ankitguptaindia As per our discussion, on the order thank you page, the product ID returning to the filter is not an integer, making the condition false.

if ( 123 === $product_id ) {
	$check_time = 'in' === $type ? '13:00' : $check_time;
	$check_time = 'out' === $type ? '14:00' : $check_time;
}

I've added a fix for it so it will always return the product ID as an integer.

@ankitguptaindia
Copy link
Member

Thanks for debugging the issue and fix. It is working fine now. @faisal-alvi

@ankitguptaindia
Copy link
Member

ankitguptaindia commented Apr 5, 2023

QA/Test Report-

Testing Environment -

  • WordPress: 6.2
  • Theme: Storefront Version: 4.2.0
  • WooCommerce - Version 7.5.1
  • PHP: 8.0.22
  • Web Server: Nginx
  • Browser: Chrome - Version 111.0.5563.64
  • OS: macOS Monterey V 12.3.1
  • Git Branch: enhancement/317

Test Results -

  • Customer's email Checked ✅
  • Order email ✅
  • Booking Calendar ✅
  • Cart ✅
  • Checkout ✅
  • Thank you page ✅
  • My Account Order Page ✅
  • My Account Bookings Page ✅
  • Single Product ✅

Functional Demo / Screencast -

https://screencast-o-matic.com/watch/c0fhIGVaxMI

Next Step- Ready to Code Review(Woo)

@ankitguptaindia ankitguptaindia removed the status: needs refresh The Issue/PR needs re-fix label Apr 5, 2023
@ankitguptaindia ankitguptaindia requested review from a team and barryhughes and removed request for a team April 5, 2023 13:54
Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

Thanks! Left some comments, but mostly optional stuff.

includes/class-wc-product-accommodation-booking.php Outdated Show resolved Hide resolved
includes/class-wc-product-accommodation-booking.php Outdated Show resolved Hide resolved
includes/class-wc-product-accommodation-booking.php Outdated Show resolved Hide resolved
*/
public static function get_check_times( $type ) {
$option = get_option( 'woocommerce_accommodation_bookings_times_settings' );
public static function get_check_times( $type, $product_id = 0 ) {
Copy link
Member

Choose a reason for hiding this comment

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

This might cause difficulties if a third party extends WC_Product_Accommodation_Booking and overrides this method (since there will be a signature mismatch). Not sure how likely that is, but it may be worth calling out in release notes/changelog.

includes/class-wc-product-accommodation-booking.php Outdated Show resolved Hide resolved
@faisal-alvi
Copy link
Member Author

@dkotter just a note for when this is merged and released: #328 (comment)

This might cause difficulties if a third party extends WC_Product_Accommodation_Booking and overrides this method (since there will be a signature mismatch). Not sure how likely that is, but it may be worth calling out in the release notes/changelog.

Alternatively, we can use func_get_args() but we should use it only if there are many third parties extending the class and I'm not sure how many are there.

Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

Looking good 👍🏼

@ankitguptaindia
Copy link
Member

Regression+Smoke Test Report-

Testing Environment -

  • WordPress: 6.2
  • Theme: Storefront Version: 4.2.0
  • WooCommerce - Version 7.7.0
  • PHP: 8.0.22
  • Web Server: Nginx
  • Browser: Chrome - Version 112

Tested with Archive File created via

php woorelease.phar build smoke-testing branch URL

  • Composer version - 2.3.5
  • npm version - 6.14.15
  • node version - v14.18.0

Please note that this plugin is tested with the build created by specified versions of Composer, Node, and NPM.

Status-

Working as expected. Ready to merge 🚀

@ankitguptaindia ankitguptaindia removed needs: regression The Issue/PR needs regresstion testing. needs: Woo review needs: UAT labels May 11, 2023
@vikrampm1 vikrampm1 modified the milestones: Future Release, 1.1.40 May 12, 2023
@vikrampm1 vikrampm1 marked this pull request as ready for review May 12, 2023 12:31
@vikrampm1 vikrampm1 merged commit 4936477 into trunk May 12, 2023
* Filter the check-in/out times for a specific product.
*
* @param string $check_time The check-in/out time stored in the database.
* @param string $type The type, check_in or check_out.

Choose a reason for hiding this comment

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

If I'm reading this correctly, $type has values of in or out, not check_in or check_out

Copy link
Member Author

Choose a reason for hiding this comment

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

@rtpHarry thanks for the report. We will fix it along with future fixes.

@jeffpaul jeffpaul deleted the enhancement/317 branch June 20, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants