Add the french La Poste delivery service #104
Conversation
|
I just now thought that it might be good to get more example data and see if this implem can be used for all laposte, colissimo and chronopost. But I'm not sure the response is exactly the same for all of them... |
guillaumeboehm
force-pushed
the
laposte_fr
branch
from
2588f18 to
abdb781
Compare
|
also, i'm not sure if another (caught) exception is a better alternative to just crashing. with a37216d, getting the necessary logs to report parsing bugs is very easy |
Aah I see, yeah indeed it's probably better to get bug reports that way. Right I'll remove the parsing throw. |
itsvic-dev
left a comment
itsvic-dev
left a comment
There was a problem hiding this comment.
i thought i submitted this last night. oh well
| return Parcel(shipment.idShip, history, status) | ||
| } | ||
|
|
||
| private val retrofit = Retrofit.Builder().baseUrl("https://www.laposte.fr/ssu/sun/back/suivi-unifie/").client(api_client) .addConverterFactory(api_factory).build() |
There was a problem hiding this comment.
this is all named Colissimo but it uses the La Poste tracking API? if anything it should be named LaPosteDeliveryService, maybe with an alias to Colissimo in Core.
also, please fix the formatting here.
There was a problem hiding this comment.
For the naming, it's a tricky situation.
LaPoste is the main state organism that handles deliveries, and it has a bunch of sub-organisms, Colissimo being one of them.
So the api hosting is done directly with laposte, but the problem is I am not sure whether the api responses are one to one between a Colissimo parcel and a Chronopost parcel for example (lowusage touched on it here also #56 (comment)).
So it's your call. I thought it would be better to stay specific at first, and eventually broaden the scope of the class if it ends up being ok. But if you prefer using "LaPoste" naming and separate them eventually if it ends up using different responses, I'll change it.
There was a problem hiding this comment.
as it's a unified tracking API, i feel like the only differences should be status codes (haven't had a proper look at the code since last time so i forgot any impl details), but the API responses themselves would be the same as any La Poste API consumers (including their own apps) would assume the response format to be the same.
that's just my hypothesis though, i don't have any Chronopost, Colissimo or La Poste parcels/letters as i'm not in France
There was a problem hiding this comment.
Alright, I'll rename it to LaPoste and make Aliases to Colissimo and Chronopost. I'll test the different providers when I get some parcels from them.
There was a problem hiding this comment.
Chronopost does have a proper event type in the event array, which is all we need in Parcel tbh
There was a problem hiding this comment.
Indeed we do have one event, but the event structure is a bit lacking... When I tried to use it I ended up removing interesting info from the Colissimo provider, the location information I think (although this is probably not that big of a deal, probably just me that doesn't know how to deal with it in kotlin)
There was a problem hiding this comment.
no, what i mean is that for the status field of the Parcel object you can just use the code from the one single item in the event array. this would look different for Colissimo though.
There was a problem hiding this comment.
Yeah I think I get what you mean, but I didn't know how to preserve the country information for Colissimo while using the same class for both chronopost and colissimo... But again probably just a skill issue on my part 😅
There was a problem hiding this comment.
you shouldn't use the same class for both. you should split the core off into a separate object that just sends the API request and returns the raw response (to reduce code duplication), then have 2 objects ColissimoDeliveryService and ChronopostDeliveryService which call that function from your object and use that to form the Parcel object using different fields
app/src/main/java/dev/itsvic/parceltracker/api/ColissimoDeliveryService.kt
Outdated
Show resolved
Hide resolved
2 participants
This MR adds the french delivery service "La poste", it should be related to #56.
I also added a new exception that I felt made sense, but I can remove it if you feel like it doesn't fit.