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.

Sign up for GitHub

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

Jump to bottom

feat: find order by api identifier #1214

Merged
merged 5 commits into from
Dec 17, 2024
Merged

feat: find order by api identifier #1214

merged 5 commits into from
Dec 17, 2024

Conversation

joerivanveen
Copy link
Contributor

@joerivanveen joerivanveen commented Dec 5, 2024 •
edited
Loading

INT-627

@myparcel-bot
Copy link
Contributor

myparcel-bot bot commented Dec 5, 2024 •
edited
Loading

PDK upgrade summary

♻️ The following packages were upgraded (from cache):

Package Old version New version
@myparcel-pdk/admin 1.5.3 1.5.5
@myparcel-pdk/admin-component-tests 1.5.3 1.5.5
@myparcel-pdk/admin-preset-dashicons 1.5.3 1.5.5
@myparcel-pdk/admin-preset-default 1.5.3 1.5.5
@myparcel-pdk/app-builder 1.6.0 1.6.1
@myparcel-pdk/checkout 1.1.5 1.2.0
myparcelnl/pdk 2.45.0 2.48.0

@codecov Codecov
Copy link

codecov bot commented Dec 5, 2024 •
edited
Loading

❌ 19 Tests Failed:

Tests completed Failed Passed Skipped
243 19 224 0
View the top 3 failed tests by shortest run time 
src/components/pdk/components.spec.ts DropOffInput > DropOffInput > has prop element
Stack Traces | 0.006s run time 
Error: No element found.
 ❯ Module.Je ../../../node_modules/@.../vue-form-builder/dist/index.js:106:11
 ❯ Module.generateFieldId ../../../node_modules/@.../utils/forms/generateFieldId.ts:9:89
 ❯ setup .../components/pdk/WcDropOffInput.vue:52:12
 ❯ callWithErrorHandling ../../../node_modules/@.../runtime-core/dist/runtime-core.cjs.js:200:19
 ❯ setupStatefulComponent ../../../node_modules/@.../runtime-core/dist/runtime-core.cjs.js:7811:25
 ❯ setupComponent ../../../node_modules/@.../runtime-core/dist/runtime-core.cjs.js:7772:36
 ❯ mountComponent ../../../node_modules/@.../runtime-core/dist/runtime-core.cjs.js:5122:7
 ❯ processComponent ../../../node_modules/@.../runtime-core/dist/runtime-core.cjs.js:5088:9
 ❯ patch ../../../node_modules/@.../runtime-core/dist/runtime-core.cjs.js:4617:11
 ❯ ReactiveEffect.componentUpdateFn [as fn] ../../../node_modules/@.../runtime-core/dist/runtime-core.cjs.js:5232:11
src/components/pdk/components.spec.ts Image > Image > has prop height
Stack Traces | 0.006s run time 
AssertionError: expected { …(2) } to have property "height"
 ❯ ../../../node_modules/@.../src/common/runHasPropTest.ts:14:51
src/components/pdk/components.spec.ts Image > Image > has prop width
Stack Traces | 0.006s run time 
AssertionError: expected { …(2) } to have property "width"
 ❯ ../../../node_modules/@.../src/common/runHasPropTest.ts:14:51

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@joerivanveen joerivanveen mentioned this pull request Dec 5, 2024
Merged
@myparcel-bot
Copy link
Contributor

myparcel-bot bot commented Dec 5, 2024 •
edited
Loading

✅ Build successful!

You can find a link to the downloadable artifact below.

Name Link
Commit 3c2dfda
Logs https://github.com/myparcelnl/woocommerce/actions/runs/12373144620
Download https://github.com/myparcelnl/woocommerce/suites/32163015026/artifacts/2331773324

@joerivanveen joerivanveen force-pushed the feat/find-order-by-api-identifier branch from 19dbaa5 to 768046c Compare December 5, 2024 15:03
@joerivanveen joerivanveen marked this pull request as ready for review December 5, 2024 15:10
@joerivanveen joerivanveen requested a review from a team as a code owner December 5, 2024 15:10
FreekVR
FreekVR reviewed Dec 9, 2024
View reviewed changes
src/Pdk/Plugin/Repository/PdkOrderRepository.php Outdated
* Load the most recent orders to search for the apiIdentifier.
* Assuming change webhooks are fired for recent orders.
*/
$orderIds = wc_get_orders([
Copy link
Collaborator

Choose a reason for hiding this comment

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

Op basis van de DB structuur zie ik idd geen betere manier om dit te doen. Het is serialized json dus kan ook geen JSON queries doen als we dat zouden willen.

Maar ik denk wel goed om dit een soort gepagineerd te doen, zodat het ook voor grote datasets werkt.
M.a.w. recursief elke keer 300 (of dan iets minder) results ophalen, als daar geen match in zit, dan een volgende set van 300 met een offset. Totdat er < 300 results in zitten en er dus niet meer resultaten over zijn, dan kun je de debug melding tonen en null returnen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Het gedrag is nu: als het te oud is, doen we geen update meer, dat was wel bewust (werkt in Magento ook zo). Maar ik vind het ook goed om de orders door te lopen tot een match is gevonden of er geen orders meer zijn. Ik zal het aanpassen.

@myparcel-bot myparcel-bot bot added the approved (Auto) label Dec 11, 2024
@joerivanveen joerivanveen merged commit a0f76cb into main Dec 17, 2024
7 checks passed
@joerivanveen joerivanveen deleted the feat/find-order-by-api-identifier branch December 17, 2024 15:04
myparcel-bot bot added a commit that referenced this pull request Dec 17, 2024
@myparcel-bot
chore(release): 5.1.0 [skip ci]
29b0974 
## [5.1.0](v5.0.0...v5.1.0) (2024-12-17)

### ⚡ Performance Improvements

* properly externalize delivery options dependency ([9c332e1](9c332e1))

### 🐛 Bug Fixes

* make filters work correctly ([#1189](#1189)) ([239878a](239878a))
* **settings:** hide delivery options position setting when blocks checkout is enabled ([#1188](#1188)) ([b6115c9](b6115c9)), closes [#1189](#1189)
* **webhooks:** prevent webhook callback never being executed ([#1187](#1187)) ([d8cf0bb](d8cf0bb))

### ✨ New Features

* **backend:** add download logs action ([#1175](#1175)) ([ee4cecb](ee4cecb))
* find order by api identifier ([#1214](#1214)) ([a0f76cb](a0f76cb))
@myparcel-bot
Copy link
Contributor

myparcel-bot bot commented Dec 17, 2024

🎉 This PR is included in version 5.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GravendeelJochem GravendeelJochem GravendeelJochem approved these changes

@FreekVR FreekVR FreekVR approved these changes

Assignees
No one assigned
Labels
approved (Auto) released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@joerivanveen @FreekVR @GravendeelJochem