Conversation
|
we are using INTEGER as type for the db but saving floating numbers at least for rooms and size.
|
|
@strech345 why did you close the pr? |
i really dont know why. I only changed to draft. But dont know why i close it. |
|
So should I review it now or is this still a draft? |
Would be nice if you can read my comments here and give me some Feedback. Also you can Start reviewing it. I would only extend it like explained. |
Tbh, I don't see an issue with this...?
|
|
|
||
| function normalize(o) { | ||
| const size = o.size || '--- m²'; | ||
| const parts = (o.tags || '').split('·').map((p) => p.trim()); |
orangecoding
left a comment
orangecoding
left a comment
There was a problem hiding this comment.
@strech345 I have not (yet) checked out your fork and ran it on my machine, bur so far I have 4 comments. Let me know when I should check out the whole thing
| return listings.map(this._providerConfig.normalize); | ||
| return listings.map((listing) => { | ||
| const normalized = this._providerConfig.normalize(listing); | ||
| // TODO: every provider should return price, size and rooms in type number. This makes it more strong and strict of the provider output. String formats like "m², Zi,..." should not be part and can be added on fe or massages. Move this logic into the provider-specific normalize function. |
There was a problem hiding this comment.
Can you make sure this is done in the pr?
| // like for kleinanzeigen we have tags (includes multiple fields) but will be than extract at normalize, and deleted because its only internal used. | ||
| // I would suggest that we define a standard list like (id, price, rooms, size, title, link, description, address, image, url) | ||
| // it might be that some of this props value is null, wich is ok without id, link, title | ||
| // Also this might be not needed when using typings with typescript. I would suggest to move the whole project to typescript to have save typings. |
There was a problem hiding this comment.
I don't like typescript for various reasons which would take way to long to explain here, but Fredy won't move to Typescript ;)
| const keys = Object.keys(this._providerConfig.crawlFields); | ||
| // i removed it because crawlFields might be different than fields which are required. | ||
| // like for kleinanzeigen we have tags (includes multiple fields) but will be than extract at normalize, and deleted because its only internal used. | ||
| // I would suggest that we define a standard list like (id, price, rooms, size, title, link, description, address, image, url) |
There was a problem hiding this comment.
I get this, but I think what we then should do is letting each provider define their list of required params. A standard function that is available in every provider which returns an array of required keys..
| // it might be that some of this props value is null, wich is ok without id, link, title | ||
| // Also this might be not needed when using typings with typescript. I would suggest to move the whole project to typescript to have save typings. | ||
| //const keys = Object.keys(this._providerConfig.crawlFields); | ||
| const keys = ['id', 'link', 'title']; |
There was a problem hiding this comment.
We should not have a fixed list in here.
|
@strech345 You probably want to merge Master into your branch as I have just pushed a pretty big change. (There's a new migration as well, so you must increase the number of yours) |
|
Ok, will take some time this week is full 😉 |
2 participants
listings can be filtered by specs