-
-
Notifications
You must be signed in to change notification settings - Fork 120
🕵️ More immoscout details #258
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
base: master
Are you sure you want to change the base?
Conversation
- Added more details to immoscout api - description is now populated with a lot of data from the expose using app API - You can ignore certificates, if deploying locally and using the http notification adapter - More details for the test call/example for easier testing + placeholder image + actual values + address (famous Erika Mustermans address see https://de.wikipedia.org/wiki/Mustermann) - Grater timeout for geocode since the api is sometimes slow in germany - uiElement, type boolean, now has a label as well
| ); | ||
| } | ||
|
|
||
| async function pushDetails(listing) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about this quite honestly, but refrained from doing it, because that creates a o^n amount of pings against the api and thus increases the likelihood of being detected as malicious by a significant fraction. I know some users are keen to get a little more infos, but quite honestly, this would only work for immoscout anyway, so I'm not sure I want to take the risk of them shutting down their api for us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about a toggle to use this feature or not - maybe you dont even want details like when you use fredy for tracking when a new apartment is listed. I guess I would be quiet annoyed to receive a bulk of data in my messages when just wanting to know there is something new.
Will change that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you 'add' ' fields' to adapters? The config doesnt really seem like it. Could make it a setting in settings instead 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mhm. indeed, the toogle (or setting) is a good idea. I've just recently introduced "user specific settings", so we might want to add a setting for the user where they can enable it. We must make sure however to put a big warning message on top, that due to the ^n amount of calls, it's more likely to get blocked.
Req: - using logger - using node-fetch Extra: - boolean input fields will trigger the validate check, because they are set undefined at first - setting them to false if they are undefined now - added more data to the description (phone number and name of the agent)
✅ Testrun completed - no errors
✅ Lint no errors (not from files I changed at least)
✅ Usefull for local deployment (guess not the only one)
✅ Usefull if you want more details from immoscout
✅ "Reverse Engineered" App for "no bot protection" expose endpoint