Add Api tests, Fix coverage issue #2376
Conversation
WalkthroughThe changes encompass the addition of new test cases, coverage tags, and modifications to existing schemas and endpoints across several files. New functionality related to announcements, linked products, reverse withdrawals, and shipping classes has been introduced. Additionally, corrections were made to variable names and schema definitions to enhance data structure and type definitions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant Database
User->>API: Request to create reverse withdrawal
API->>Database: Store reverse withdrawal transaction
Database-->>API: Confirm transaction stored
API-->>User: Return success response
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (7)
tests/pw/tests/api/shippingStatus.spec.ts (2)
1-7: LGTM! Coverage tags updated correctly.The changes to the coverage tags are good:
- Simplified regex patterns improve readability.
- Added DELETE method coverage tag for completeness.
Consider moving the comment about the DELETE method not existing in the Dokan API to the same line as the coverage tag for better readability:
-//COVERAGE_TAG: DELETE /dokan/v1/shipping-status/orders/(?P<order_id>[\d]+)/shipment/(?P<id>[\d]+) -// delete method doesn't exists on dokan +//COVERAGE_TAG: DELETE /dokan/v1/shipping-status/orders/(?P<order_id>[\d]+)/shipment/(?P<id>[\d]+) // delete method doesn't exist in Dokan API
DELETE operations detected without corresponding test cases.
The codebase includes the following DELETE methods:
delete_item_permissions_checkanddelete_iteminProductAttributeTermsController.phpdelete_item_permissions_checkinProductAttributeController.phpdelete_item_permissions_checkanddelete_iteminWithdrawController.phpIt's recommended to add test cases for these DELETE operations to ensure comprehensive coverage and validate their behavior.
Analysis chain
Line range hint
9-71: Verify DELETE operation status and consider adding a test case if applicable.The test cases cover GET, POST, and PUT operations as mentioned in the coverage tags. However, there's no test case for the DELETE operation.
Could you please confirm if the DELETE operation is truly not implemented in the Dokan API? If it is implemented or planned for the near future, consider adding a test case for it. If it's not implemented and not planned, the current set of test cases is complete.
To verify the implementation status, you can run the following command:
If the search returns results, it might indicate that the DELETE method is implemented, and we should add a test case for it.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any DELETE method implementation in the codebase rg --type php 'function\s+delete_item' -g 'includes/REST/*.php'Length of output: 568
tests/pw/tests/api/reverseWithdrawal.spec.ts (1)
63-69: LGTM: Well-structured test case addedThe new test case for creating a reverse withdrawal transaction is well-structured and follows the established patterns in the file. It correctly uses
apiUtilsmethods, validates the response, and uses appropriate authentication.Consider adding a comment explaining the purpose of this test case, similar to other tests in the file. This would improve readability and maintainability. For example:
// Test creating a reverse withdrawal transaction with valid data test('create a reverse withdrawal transaction', { tag: ['@lite'] }, async () => { // ... (rest of the test case) });tests/pw/tests/api/announcements.spec.ts (1)
Line range hint
91-112: Existing tests align well with new coverage tags.The existing test cases for announcement notices (get, update, delete) align perfectly with the newly added coverage tags. This ensures good test coverage for these endpoints.
For consistency, consider renaming the "update an announcement notice" test to "create/update an announcement notice" since it uses a POST request, which could be used for both creating and updating.
Consider applying this minor change for improved clarity:
- test('update an announcement notice', { tag: ['@pro'] }, async () => { + test('create/update an announcement notice', { tag: ['@pro'] }, async () => {tests/pw/tests/api/products.spec.ts (1)
144-150: LGTM: New test case added for linked products.The new test case for retrieving linked products is well-structured and follows the general pattern of other tests in the file. It correctly uses the PRODUCT_ID environment variable and performs appropriate assertions.
For consistency with other tests in this file, consider moving the
apiUtilsinitialization to abeforeAllhook within atest.describeblock, similar to the structure used for other API version tests. This would also allow for proper cleanup in anafterAllhook.Here's a suggested refactor:
test.describe('linked products api test v2', () => { let apiUtils: ApiUtils; test.beforeAll(async () => { apiUtils = new ApiUtils(await request.newContext()); }); test.afterAll(async () => { await apiUtils.dispose(); }); test('get all linked products', { tag: ['@pro', '@v2'] }, async () => { const [response, responseBody] = await apiUtils.get(endPoints.getAllLinkedProducts(PRODUCT_ID)); expect(response.ok()).toBeTruthy(); expect(responseBody).toBeTruthy(); expect(responseBody).toMatchSchema(schemas.productsSchema.linkedProductsSchema); }); });This structure ensures proper setup and teardown of resources for the test case.
tests/pw/utils/payloads.ts (1)
3953-3958: LGTM: createReverseWithdrawal object is well-structured.The object properties are clearly defined and include helpful comments for the
trn_typeoptions.Consider using a number type for the
creditproperty instead of a string, unless string is specifically required by the API:- credit: '100', + credit: 100,tests/pw/utils/schemas.ts (1)
Line range hint
2257-2269: Consider coercing numeric and date fields inreverseWithdrawalTransactionsSchemaThe fields
debit,credit,balance, andtrn_dateinreverseWithdrawalTransactionsSchemaare defined asz.string().or(z.number())andz.string(). If these fields are expected to represent numeric and date values, consider usingz.coerce.number()andz.coerce.date()respectively to ensure consistent data types.Apply this diff to update the fields:
- trn_date: z.string(), // Assuming trn_date is a string representation of a date + trn_date: z.coerce.date(), - debit: z.string().or(z.number()), + debit: z.coerce.number(), - credit: z.string().or(z.number()), + credit: z.coerce.number(), - balance: z.string().or(z.number()), + balance: z.coerce.number(),
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- tests/pw/tests/api/announcements.spec.ts (1 hunks)
- tests/pw/tests/api/products.spec.ts (2 hunks)
- tests/pw/tests/api/reverseWithdrawal.spec.ts (2 hunks)
- tests/pw/tests/api/shippingStatus.spec.ts (1 hunks)
- tests/pw/tests/api/withdraws.spec.ts (1 hunks)
- tests/pw/utils/apiEndPoints.ts (3 hunks)
- tests/pw/utils/payloads.ts (1 hunks)
- tests/pw/utils/schemas.ts (8 hunks)
Additional comments not posted (14)
tests/pw/tests/api/reverseWithdrawal.spec.ts (2)
5-5: LGTM: Coverage tag added correctlyThe new coverage tag for the POST endpoint of reverse withdrawal transactions is correctly formatted and placed. This addition improves test coverage tracking.
Line range hint
1-85: Overall assessment: Excellent additions to test coverageThe changes in this file enhance the test suite for reverse withdrawal functionality. The new coverage tag and test case are well-implemented and consistent with the existing code structure. These additions will help ensure the reliability of the reverse withdrawal transaction creation process.
tests/pw/tests/api/announcements.spec.ts (1)
8-10: LGTM! New coverage tags added correctly.The new coverage tags for announcement notice endpoints (GET, POST, DELETE) have been added correctly and are consistent with the existing tag format. This is a good addition to track API coverage for these endpoints.
To ensure that the test suite covers these new endpoints, please run the following script:
This script will help confirm that corresponding test cases have been implemented for these new endpoints.
Verification successful
Tests for the new announcement notice endpoints are present.
The executed shell script confirmed that test cases for GET, POST, and DELETE announcement notice endpoints are implemented in
tests/pw/tests/api/announcements.spec.ts. The test suite adequately covers these new endpoints.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the presence of tests for the new announcement notice endpoints. # Test: Search for test cases related to the new endpoints rg --type typescript 'test\(.*announcement notice' tests/pw/tests/api/announcements.spec.tsLength of output: 128
Script:
#!/bin/bash # Description: Verify the presence of tests for the new announcement notice endpoints. # Test: Search for test cases related to the new endpoints using the correct file type. rg --type ts 'test\(.*announcement notice' tests/pw/tests/api/announcements.spec.tsLength of output: 311
tests/pw/tests/api/withdraws.spec.ts (2)
Line range hint
114-120: LGTM: Test cases implemented for new withdraw charges endpoints.The test cases for 'get all withdraw method charges' and 'get withdraw charge details' are well-implemented:
- They use appropriate API calls to the new endpoints.
- The responses are checked for successful status codes.
- The response bodies are validated against the corresponding schemas.
This ensures good coverage for the newly added endpoints.
Also applies to: 122-128
9-10: LGTM: New coverage tags added for withdraw charges endpoints.The addition of coverage tags for
GET /dokan/v1/withdraw/chargesandGET /dokan/v1/withdraw/chargeis appropriate and aligns with the existing structure of the file.Let's verify if corresponding test cases exist for these new endpoints:
Verification successful
Action Required: Correct the shell scripts to search for the new withdraw charges test cases.
Action Required: Verify the existence of test cases for the new withdraw charges endpoints by listing all test functions in the file.
Action Required: Extract detailed test case descriptions to verify coverage for new withdraw charges endpoints.
: Coverage tags for
GET /dokan/v1/withdraw/chargesandGET /dokan/v1/withdraw/chargehave corresponding test cases (get all withdraw method chargesandget withdraw charge details). The tests adequately cover the newly tagged endpoints.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for test cases related to the new withdraw charges endpoints # Test: Search for test cases using the new endpoints rg --type typescript 'test\(.*getAllWithdrawMethodCharges' tests/pw/tests/api/withdraws.spec.ts rg --type typescript 'test\(.*getWithdrawCharge' tests/pw/tests/api/withdraws.spec.tsLength of output: 255
Script:
# Description: Corrected shell scripts to check for test cases related to the new withdraw charges endpoints # Test: Search for test cases using the new endpoints without specifying an unrecognized file type rg 'test\(.*getAllWithdrawMethodCharges' tests/pw/tests/api/withdraws.spec.ts rg 'test\(.*getWithdrawCharge' tests/pw/tests/api/withdraws.spec.tsLength of output: 149
Script:
# Description: List all test function names in withdraws.spec.ts to identify relevant test cases rg 'test\(' tests/pw/tests/api/withdraws.spec.ts --no-filename --only-matching --unrestrictedLength of output: 161
Script:
# Description: Extract all test case descriptions in withdraws.spec.ts to identify relevant test cases rg "test\(['\"](.*?)['\"]" tests/pw/tests/api/withdraws.spec.ts --only-matching --no-filename --color neverLength of output: 442
tests/pw/tests/api/products.spec.ts (1)
25-25: LGTM: New coverage tag added correctly.The new coverage tag for the linked products endpoint is properly formatted and placed alongside other similar tags. This addition helps maintain accurate test coverage documentation.
tests/pw/utils/apiEndPoints.ts (3)
219-219: LGTM: New endpoint added for reverse withdrawal transactions.The
createReverseWithdrawalTransactionsendpoint has been correctly added to theendPointsobject. It follows the existing naming conventions and URL structure patterns.
304-305: LGTM: New endpoint added for retrieving linked products.The
getAllLinkedProductsendpoint has been correctly added to theendPointsobject. It follows the existing naming conventions and properly includes theproductIdas a parameter. The URL structure with a query parameter is appropriate for this type of request.
514-521: LGTM: Shipping class endpoints reorganized.The shipping class endpoints have been appropriately moved to the
wcobject, which is a more suitable location for WooCommerce-specific endpoints. The move maintains consistent naming conventions and URL structures. All necessary CRUD operations and the batch update endpoint are included, providing a complete set of operations for shipping classes.tests/pw/utils/payloads.ts (2)
3951-3952: LGTM: Random number generation looks good.The
randomNumberproperty is correctly implemented using faker.js to generate a random integer between 2 and 100.
3951-3958: Overall, the code looks good and is well-structured.Both the
randomNumberproperty andcreateReverseWithdrawalobject are implemented correctly. The random number generation is straightforward, and the reverse withdrawal object provides a clear structure for creating transactions. The suggested minor improvement for thecreditproperty type is optional and depends on the API requirements.tests/pw/utils/schemas.ts (3)
1705-1705: IncludelinkedProductsSchemain exports if necessaryThe
linkedProductsSchemais defined asz.array(linkedProductSchema). Ensure that this new schema is correctly exported and utilized where necessary in the codebase.
2238-2239: Include required fields increateReverseWithdrawalTransactionschemaThe
createReverseWithdrawalTransactionschema definestrn_idasz.string().or(z.number()). Verify that all necessary fields for creating a reverse withdrawal transaction are included, and consider adding validation for other required fields if applicable.Also applies to: 2275-2277
182-186: Verify theattributesfield type inlinkedProductSchemaThe
attributesfield inlinkedProductSchemais defined asz.array(productAttributeSchema).or(z.object({ pa_sizes: z.any() })). Please confirm that this accurately represents the possible data structures returned by the API. Ifattributescan be both an array and an object, consider usingz.union([z.array(productAttributeSchema), z.object({ pa_sizes: z.any() })])for clarity.Run the following script to verify the
attributesfield usage:
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation