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

Adds location id to order table #91

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Mrkbingham
Copy link

Add location id to the order table

Since all Square orders (created using the Square facade) require a location ID to be added, storing that information will be useful for tracking orders on a per-store basis when integrating this library into other applications. The following changes apply here:

  1. There is a migration to add location_id to the orders table
  2. The order of the _saveOrder is slightly altered - first checking if a locationId is present, and then throwing an error if it's not. This prevents orders with no location ID from being saved, which may be notably different from the original use-case under which it was built.
  3. This adds the square location env variable to the Order model factory

@NikolaGavric94
Copy link
Owner

In case of single store for all products (one location_id) will the code read from env and insert it if the one isn't provided as an input?

@Mrkbingham
Copy link
Author

In case of single store for all products (one location_id) will the code read from env and insert it if the one isn't provided as an input?

No, it currently does not. There is an existing check for location_id during the charge process though:
https://github.com/NikolaGavric94/laravel-square/blob/master/src/SquareService.php#L239

And since it's a required parameter for the setOrder function, it seems unlikely there would be many scenarios where locationId would not be available/passed through.
https://github.com/NikolaGavric94/laravel-square/blob/master/src/SquareService.php#L450

@NikolaGavric94
Copy link
Owner

There seems to be some errors with current tests, could you take a look and make a fix for it?

@Mrkbingham
Copy link
Author

@NikolaGavric94 Just to confirm - it looks like, outside of the syntax issues, every other error was getting thrown likely due to the fact that SQUARE_ORDER_NAMESPACE was not defined during the test run. In particular, this references line 18:
image

And in line 18 here references the namespace config:
Screenshot 2024-08-06 at 4 35 59 PM

Are there any issues with the secrets in the github workflow file? This branch is passing locally for me when I run the same command in the main.yml file:
vendor/bin/phpunit --migrate-configuration && vendor/bin/phpunit --coverage-text --coverage-clover build/logs/clover.xml

@Mrkbingham
Copy link
Author

Link to successful PHPUnit pipeline: https://github.com/Mrkbingham/laravel-square/pull/24/checks

@NikolaGavric94
Copy link
Owner

Not to be merged yet. I'll keep it approved for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants