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

Contacts and import orders endpoints #31

Merged
merged 6 commits into from
Jun 22, 2023

Conversation

MartinMystikJonas
Copy link
Contributor

@MartinMystikJonas MartinMystikJonas commented May 25, 2023

Navazuje na #30

Doplněny endpointy pro /contacts a /import-orders

Opravěné nějaké nefunkční věci

Možnost s CustomField číst options - aktuálně nefungovalo vůbec + doplněný value objekt

Změna logiky JSON -> Model, aby se dal stejný model použít pro write i read.

Trochu pročištěné a opravené testy.

@MartinMystikJonas MartinMystikJonas force-pushed the endpoints branch 3 times, most recently from 8eba7c0 to a5d05ff Compare May 30, 2023 12:45
@pionl
Copy link
Owner

pionl commented Jun 21, 2023

Prosím o rebase :)

@MartinMystikJonas
Copy link
Contributor Author

@pionl rebased

{
$this->select = $select;
return $this;
InvalidFormatException::checkAllowedValues($select, CustomFieldDefinition::SELECT_FIELDS);
Copy link
Owner

Choose a reason for hiding this comment

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

Nechceš místo overridu (které máme nyní na více implementacích) dát abstraktní funkci která vynutí aby si vrátil seznam povolených select fieldů a v té abstraktní třídě volat ty checky?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Takhle to bylo řešeno v původní implementaci tak jsme to zachoval, ale je fakt že ta abstraktní fce by asi byla lepší

Copy link
Owner

Choose a reason for hiding this comment

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

Můžeme se změnit v budoucnu :) díky!

Copy link
Owner

@pionl pionl left a comment

Choose a reason for hiding this comment

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

Díky, nemám poznámek, super :)

@pionl pionl merged commit fb2062a into pionl:master Jun 22, 2023
@pionl
Copy link
Owner

pionl commented Jun 22, 2023

dej vědět kdy chceme udělat release verzi :)

@MartinMystikJonas
Copy link
Contributor Author

Asi bych jeste projel ty model tridy a predelal to vsechno na gettery/settery. Jinak to asi pak muzem releasnout. Tuhle verzi uz mam v produkci.

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