-
Notifications
You must be signed in to change notification settings - Fork 3
Feature Countries #54
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive support for the Countries API endpoint to the Bexio SDK, including all CRUD operations (Create, Read, Update, Delete) and search functionality. The PR also refactors the Items enum from OrderByEnum to ItemsOrderByEnum to avoid naming conflicts with the new Countries enum, and adds error suppression to test fixture cleanup operations for better test reliability.
Key Changes
- Added full Countries API support with request classes, DTOs, enums, tests, and documentation
- Renamed
Items\OrderByEnumtoItems\ItemsOrderByEnumto prevent naming conflicts - Added
@error suppression operator tounlink()calls in test files for more reliable fixture cleanup
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Requests/Items/SearchItemsRequestTest.php | Added error suppression to fixture cleanup |
| tests/Requests/Items/FetchAListOfItemsRequestTest.php | Added error suppression to fixture cleanup |
| tests/Requests/Items/CreateItemRequestTest.php | Added error suppression to fixture cleanup |
| tests/Requests/Countries/SearchCountriesRequestTest.php | New test for Countries search endpoint |
| tests/Requests/Countries/FetchAListOfCountriesRequestTest.php | New test for fetching list of countries |
| tests/Requests/Countries/FetchACountryRequestTest.php | New test for fetching a single country |
| tests/Requests/Countries/EditACountryRequestTest.php | New test for editing a country |
| tests/Requests/Countries/DeleteACountryRequestTest.php | New test for deleting a country |
| tests/Requests/Countries/CreateCountryRequestTest.php | New test for creating a country |
| tests/Fixtures/Saloon/* | Updated test fixtures with new API responses |
| src/Requests/Items/SearchItemsRequest.php | Updated to use renamed ItemsOrderByEnum |
| src/Requests/Items/FetchAListOfItemsRequest.php | Updated to use renamed ItemsOrderByEnum |
| src/Requests/Countries/SearchCountriesRequest.php | New request class for searching countries |
| src/Requests/Countries/FetchAListOfCountriesRequest.php | New request class for fetching countries list |
| src/Requests/Countries/FetchACountryRequest.php | New request class for fetching a single country |
| src/Requests/Countries/EditACountryRequest.php | New request class for editing a country |
| src/Requests/Countries/DeleteACountryRequest.php | New request class for deleting a country |
| src/Requests/Countries/CreateCountryRequest.php | New request class for creating a country |
| src/Enums/Items/ItemsOrderByEnum.php | Renamed from OrderByEnum to avoid conflicts |
| src/Enums/Countries/CountriesOrderByEnum.php | New enum for country ordering options |
| src/Dto/Countries/CreateEditCountryDTO.php | New DTO for creating/editing countries |
| src/Dto/Countries/CountryDTO.php | New DTO for country data |
| README.md | Added documentation for Countries API endpoints and updated Items enum reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (shouldResetFixtures() && file_exists($fixturePath)) { | ||
| @unlink($fixturePath); | ||
| } |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent file existence check before unlink. Unlike other Country test files (FetchAListOfCountriesRequestTest.php, CreateCountryRequestTest.php, etc.) and Item test files that use @unlink($fixturePath) directly, this test adds an additional file_exists($fixturePath) check. This is unnecessary since @unlink() already suppresses the warning if the file doesn't exist. For consistency with the rest of the codebase, remove the file_exists() check.
Suggested change:
if (shouldResetFixtures()) {
@unlink($fixturePath);
}
No description provided.