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 Shipping Max time column in gla_shipping_times Table #2520

Merged
merged 2 commits into from
Aug 14, 2024

Conversation

jorgemd24
Copy link
Contributor

@jorgemd24 jorgemd24 commented Aug 13, 2024

Changes proposed in this Pull Request:

Part of pcTzPl-2qP-p2

This PR updates the DB to support minimum and maximum shipping times. We'll keep the existing time column (which will be used as min time) for backwards compatibility and add a new column called max_time, which will be populated with the current values from the time column for existing users.

Once the UI is set up to handle max and min times, we’ll update the backend to manage these new columns accordingly.

Screenshots:

Detailed test instructions:

  1. Edit your Free Listing to have different countries and shipping times.
  2. Update the Migration class to your current GLA version: https://github.com/woocommerce/google-listings-and-ads/pull/2520/files#diff-898d6eb44e5ad0a4ef4f1b8ac7af29f6d561f5ddfb30b1ab79398dd81a629df8R46 for example: 2.8.1
  3. Adjust the gla_db_version option to an older version, such as 2.8.0, so that the migration class is triggered.: UPDATE wp_options SET option_value = '2.8.0' WHERE option_name = 'gla_db_version';
  4. Refresh the GFW page.
  5. Check the DB and see that there is a new column called max_time and is populated with the values used in time.
  6. To simulate a new installation run the following queries:
DROP TABLE wp_gla_shipping_times;
DELETE FROM wp_options WHERE `option_name` = 'gla_db_version';
  1. Refresh and see that a new column is added.

Additional details:

I didn’t include tests since this code will only run once.

Changelog entry

@github-actions github-actions bot added type: enhancement The issue is a request for an enhancement. changelog: add A new feature, function, or functionality was added. labels Aug 13, 2024
Copy link

codecov bot commented Aug 13, 2024

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Please upload report for BASE (update/shippings-settings-phase-1@9f669d4). Learn more about missing BASE report.

Files Patch % Lines
src/DB/Migration/Migration20240813T1653383133.php 0.0% 9 Missing ⚠️
...nternal/DependencyManagement/DBServiceProvider.php 0.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                         Coverage Diff                         @@
##             update/shippings-settings-phase-1   #2520   +/-   ##
===================================================================
  Coverage                                     ?   65.0%           
  Complexity                                   ?    4585           
===================================================================
  Files                                        ?     476           
  Lines                                        ?   17898           
  Branches                                     ?       0           
===================================================================
  Hits                                         ?   11626           
  Misses                                       ?    6272           
  Partials                                     ?       0           
Flag Coverage Δ
php-unit-tests 65.0% <0.0%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/DB/Table/ShippingTimeTable.php 53.8% <ø> (ø)
...nternal/DependencyManagement/DBServiceProvider.php 0.0% <0.0%> (ø)
src/DB/Migration/Migration20240813T1653383133.php 0.0% <0.0%> (ø)

* @return void
*/
public function apply(): void {
if ( $this->shipping_time_table->exists() && ! $this->shipping_time_table->has_column( 'max_time' ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the table installation happens earlier (even if the table already exists) and is handled by the delta function, the table should already be updated, so this code likely won't run. However, I think it's a good idea to keep it as a precaution.

$this->wp->db_delta( $this->get_install_query() );

https://developer.wordpress.org/reference/functions/dbdelta/

@jorgemd24 jorgemd24 self-assigned this Aug 13, 2024
@jorgemd24 jorgemd24 requested a review from a team August 13, 2024 17:48
@jorgemd24 jorgemd24 marked this pull request as ready for review August 13, 2024 17:48
@jorgemd24 jorgemd24 changed the title Add/max time column db Add Shipping Max time column in gla_shipping_times Table Aug 13, 2024
Copy link
Contributor

@puntope puntope left a comment

Choose a reason for hiding this comment

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

LGTM

Screenshot 2024-08-14 at 10 47 53

I see the new max_time created and populated correctly.

@jorgemd24 jorgemd24 merged commit 9edf689 into update/shippings-settings-phase-1 Aug 14, 2024
14 checks passed
@jorgemd24 jorgemd24 deleted the add/max-time-column-db branch August 14, 2024 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants