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

Add support for Zásilkovna (+ little bit of transients for JSONs) #120

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

Conversation

oerdnj
Copy link

@oerdnj oerdnj commented Nov 7, 2016

Hi,

I have added support for Zásilkovna (basically cut&paste from Uloženka) and added transients for loaded and parsed jsons.

Please merge.

@pavelevap
Copy link
Owner

Wow, thank you! 👍

I will test it during this week, but it looks good. Maybe we could also merge two similar json loader files?

Transients are a very good idea :-)

Issue: #15

@lukasprelovsky
Copy link
Contributor

WAU 👍

@oerdnj
Copy link
Author

oerdnj commented Nov 7, 2016

There's more stuff that's just cut&paste, so it could have a common class, but my PHP-fu is very rusty as I try to actively avoid writing code in PHP, and I needed something for my wife's shop (https://vseprochleba.cz) finished before my attention turns into some other direction :). Feel free to modify as it suits you, but I am big fan of "release early, release often" and there's plenty of time for rebasing later.

@oerdnj
Copy link
Author

oerdnj commented Nov 7, 2016

One more commit that will allow storing a local copy of Uloženka and Zásilkovna logos inside local WP instance (if you don't want to ping their logos every time your customer enters cart...)

You can see the running instance at https://vseprochleba.cz/

'default' => 'Zásilkovna',
'css' => 'width: 300px;'
),
'zasilkovna_id-obchodu' => array(
Copy link

Choose a reason for hiding this comment

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

ID obchodu v kontextu zásilkovny znamená něco trochu jiného (textový řetězec, který identifikuje odesilatele při podávání přes API). Můžete to prosím upravit na API klíč? V zásilkovně to tak máme vedené všude a předejdeme zmatkům :)

Copy link
Author

Choose a reason for hiding this comment

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

Ok, změněno.

@oerdnj
Copy link
Author

oerdnj commented Nov 8, 2016

There's one more thing I thought it's worth doing. The Json classes are really useless as they are as only one function is really used. The basic class should be rewritten as Iterator and the Uloženka and Zásilkovna classes should just redefine the method that constructs URL and iterates through data.

@pavelevap
Copy link
Owner

@oerdnj: Yes, Iterator is also great possibility, I would welcome some concept :-) I planned to have only one simple class (as before this pull request) and with any new shipping method, I could simply use $params to pass API endpoint and shipping ID (these are probably the only information for different shipping methods).

And one more thing, configurable logo URLs is great idea, but I am not sure if we should require users to download logo, upload it, find exact URL and insert it into text field. It is quick and easy for us, but not for many users (also with possible problems when switching eshop to HTTPS, etc). I planned to download it with wp_remote_get(), save it into Media library automatically and use only ID of this file. And in the end it could be configurable by upload field (user do not add URL in text format, but click on button and select/upload another file). But I did not have time to do it, so it was simply loaded from external source :-)

@romanrehacek
Copy link

@pavelevap Zdravim, chcem sa spytat, ako je to s touto upravou. Kedy vidite, ze by mohla byt sucastou pluginu? Zakaznik by rad zasielkovnu vyuzival a nechcem mu davat dalsi plugin. Vdaka.

@oerdnj
Copy link
Author

oerdnj commented Dec 6, 2016

@romanrehacek Kvůli tomu, že se tohle moc nehýbe, jsem stvořil WooCommerce Zásilkovna plugin: https://github.com/oerdnj/woocommerce-zasilkovna

Nicméně ten nový potřebuje WC 2.6+ kvůli tomu, že podporuje Shipping Zones (tj. pro každou shipping zónu se dá nastavit samostatná cena).

@romanrehacek
Copy link

@oerdnj Super vdaka. Vyskusam.

@pavelevap
Copy link
Owner

Díky :-) Nehýbalo se to proto, že jsem zatím neměl čas to přepsat a dosavadní PR jsem v této podobě integrovat kompletně nechtěl (viz výše zmiňované pole s URL adresou a sloučení s Uloženkou). Takže Zásilkovna součástí pluginu České služby určitě bude, ale ještě tam musím pár věcí dodělat... Docela se mi na novém pluginu líbí debug mode a nechám se asi inspirovat i Iteratorem a transienty :-)

@oerdnj
Copy link
Author

oerdnj commented Dec 6, 2016

Nebylo by lepší, kdyby se České Služby obecně rozpadly do více pluginů? A ceske-sluzby byl jenom plugin, který by doporučoval instalaci ostatních (podobně jako to dělá Virtue theme)?

@pavelevap
Copy link
Owner

To je také hodně zajímavý nápad :-) Už jsem o tom také přemýšlel, ale nejsem si jist, jak to celé udělat, aby zase nebylo nutné mít pro každou část samostatný plugin (což je někdy zbytečné). Dosavadní záměr byl takový, že současné funkce rozsekám do jednotlivých souborů a v administraci vznikne nějaká aktivační stránka, kde je bude možné aktivovat/načítat (nyní tam jsou vlastně stejně aktivační checkboxy). Bylo by hezké, kdyby tam šlo přidat i další (externí) pluginy a případně umožnit jejich instalaci (třeba i z Githubu). Nápady vítány :-)

Virtue theme neznám, podívám se. Je to tato šablona?

@oerdnj
Copy link
Author

oerdnj commented Dec 7, 2016

@pavelevap Ano, je to ona. (Resp. na vseprochleba.cz používáme to "Pro" variantu kvůli breadcrumbs, ale jinak je to asi nejlepší šablona pro Woo, na kterou jsme narazili.)

@oerdnj
Copy link
Author

oerdnj commented Dec 7, 2016

Ta aktivace těch pluginů je v virtue/lib/virtuetoolkit-activate.php

@romanrehacek
Copy link

No mne sa tento plugin paci prave kvoli tomu, ze je vsetko v jednom. Nepotrebujem mat na kazdu funkcionalitu samostatny plugin.

Chcete teda tento pugin prekopat uplne od zakladov a spravit nanovo? Kludne vam s tym pomozem ;-)

@pavelevap
Copy link
Owner

@oerdnj: Díky, podívám se na to.

@romanrehacek: Celý ten plugin je vlastně spousta různých funkcí, jejichž zapínání je řešeno aktivačními checkboxy (např. dodací doba, XML feedy, Heureka certifikát, atd), což je zase důsledek toho, že nebylo původně zamýšleno, že toho bude obsahovat tolik :-) Vše co nyní obsahuje by samozřejmě obsahoval i nadále (nebyly by to samostatné pluginy, ale spíše doplňkové addony), jen by aktivace jednotlivých částí (addonů) probíhala přehledně na jedné stránce + byly by tam i další tipy na free pluginy, které nejsou součástí pluginu České služby, ale dají se dohledat a jsou funkční (třeba jako právě nyní Zásilkovna a další). Trochu problém ale je, že jsou některé části propojené, např. dodací doba a XML feedy, EAN a XML feedy, atd. Ale to by šlo generalizovat, stejně tam máme podporu pro vlastní řešení (např. pomocí postmeta) a vlastně by to bylo do budoucna spíše k dobru :-)

@pavelevap
Copy link
Owner

Takže šablona Virtue používá instalační systém TGM Plugin Activation. Tam by možná šlo i nastavit, že jsou jednotlivé části vlastně součástí hlavního pluginu a následně by se mohly doplnit i další doporučované pluginy (s instalačními odkazy). Zkusím si s tím trochu pohrát, to by mohlo být zajímavé (a vcelku rychlé) řešení (alespoň pro doporučování externích pluginů)...

@pavelevap pavelevap mentioned this pull request Jan 25, 2017
@matyx
Copy link

matyx commented Mar 30, 2017

Zdravím,
jak to prosím vypadá s tímto PR? Je v plánu jej mergovat do masteru?

@pavelevap
Copy link
Owner

Ano, jen dojde k některým úpravám. Nebude už vyžadován externí plugin Pay for Payment a i další možnosti (např. příplatek za dobírku) budou přímo nastavitelné v rámci pluginu České služby. Prosím ještě o trpělivost...

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.

5 participants