Update the shipping time controllers to handle the maximum shipping time

[jorgemd24]

Changes proposed in this Pull Request:

Part of pcTzPl-2qP-p2

This PR updates the shipping time controllers to allow for updating and fetching the maximum shipping time, which will be used in a second PR that I’m currently working on for the UI.

Screenshots:

Detailed test instructions:

Prerequisites

1. Follow the steps mentioned in this PR to update the database: Add Shipping Max time column in gla_shipping_times Table #2520

1. Make a GET /wc/gla/mc/shipping/times request and check that the max_time property is now included in the response.
2. Update one of the shipping times by making the following request: POST gla/mc/shipping/times and the following body: {"country_code":"ES","time":2, "max_time": 3}
3. Make the following request for one specific country: GET gla/mc/shipping/times/YOUR_COUNTRY for example GET gla/mc/shipping/times/ES and check that the max_time property is now included.

Additional details:

Changelog entry

[codecov]

Codecov Report

Attention: Patch coverage is 77.50000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 66.3%. Comparing base (8f3231f) to head (af40ca3).
Report is 148 commits behind head on update/shippings-settings-phase-1.

Files with missing lines	Patch %	Lines
...trollers/MerchantCenter/ShippingTimeController.php	85.3%	5 Missing ⚠️
src/DB/Table/ShippingTimeTable.php	0.0%	4 Missing ⚠️

Additional details and impacted files

@@                          Coverage Diff                          @@
##             update/shippings-settings-phase-1   #2590     +/-   ##
=====================================================================
+ Coverage                                 65.2%   66.3%   +1.0%     
- Complexity                                4588    4598     +10     
=====================================================================
  Files                                      803     475    -328     
  Lines                                    23012   17927   -5085     
  Branches                                  1234       0   -1234     
=====================================================================
- Hits                                     15012   11879   -3133     
+ Misses                                    7833    6048   -1785     
+ Partials                                   167       0    -167     

Flag	Coverage Δ
js-unit-tests	?
php-unit-tests	66.3% <77.5%> (+0.7%)	⬆️

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

Files with missing lines	Coverage Δ
...ers/MerchantCenter/ShippingTimeBatchController.php	92.5% <100.0%> (+0.4%)	⬆️
src/DB/Table/ShippingTimeTable.php	50.0% <0.0%> (-3.8%)	⬇️
...trollers/MerchantCenter/ShippingTimeController.php	93.6% <85.3%> (+53.4%)	⬆️

... and 327 files with indirect coverage changes

[puntope]

@jorgemd24 I see one potential issue in the controller where you can set -1 as value for time and 'max_time':

1. Request POST wp-json/wc/gla/mc/shipping/times with {"country_code":"es, "max_time": -1, time: -1 }
2. Request is successful
3. Request GET wp-json/wc/gla/mc/shipping/times/es
4. It returns max_time and time as -1

[puntope]

Another potential issue I see is that we don't require time and max_time which is overrides as 0 is you don't set them.

1. Request POST wp-json/wc/gla/mc/shipping/times with {"country_code":"es, "max_time": 1, time: 1 }
2. Request is successful
3. Request GET wp-json/wc/gla/mc/shipping/times/es
4. It returns max_time and time as 1
5. Request POST wp-json/wc/gla/mc/shipping/times with {"country_code":"es, "max_time": 2 }
6. Request is successful
7. Request GET wp-json/wc/gla/mc/shipping/times/es
8. It returns max_time 2 but time as 0 (it shouldn't be 1?)

[puntope]

Finally, one extra issue I see is that time can be set as a bigger number that max_time

1. Request POST wp-json/wc/gla/mc/shipping/times with {"country_code":"es, "max_time": 1, time: 2 }
2. Request is successful
3. Request GET wp-json/wc/gla/mc/shipping/times/es
4. It returns max_time 1 and time as 2

[jorgemd24]

Thanks, @puntope, for your helpful review! I've made the changes based on your comments. Could you please take another look? Thanks!

[puntope]

Just curious. Why we cannot just add the 'required' in the array inside get_schema_properties method ?

[jorgemd24]

That was my initial approach, but the schema is shared between the GET and POST methods. If I make time and max_time required, the UI will also need to include those query parameters when fetching the times.

[puntope]

Thanks @jorgemd24 for the adjustments validating the request.

Now LGTM

I left just one question regarding where we place the required arg.