Conversation
ajvpot
changed the title
ajvpot
changed the title
|
Love this. I'm on an extended trip right now so I'm be a bit slow for review. I will be back home and have final review on this within a couple weeks. Everything looks very good though, thank you for the contribution! |
|
Hey @juftin, hope your vacation went well! Please let me know if there's anything I can update on this. |
| # Remove items that will be templated as part of the embed. | ||
| del formatted_dict["Recreation Area"] | ||
| del formatted_dict["Booking Date"] | ||
| del formatted_dict["Booking End Date"] | ||
| del formatted_dict["Facility Name"] | ||
| del formatted_dict["Booking Link"] | ||
| del formatted_dict["Campsite Site Name"] | ||
| del formatted_dict["Campsite Loop Name"] | ||
| del formatted_dict["Recreation Area Id"] | ||
| del formatted_dict["Facility Id"] | ||
| del formatted_dict["Campsite Id"] | ||
|
|
||
| return { | ||
| "author": { | ||
| "name": f"🏕 {campsite.recreation_area}" | ||
| }, | ||
| "title": f"{campsite.facility_name} {campsite.campsite_loop_name} #{campsite.campsite_site_name}", | ||
| "description": f"{campsite.booking_date:%Y/%m/%d} to {campsite.booking_end_date:%Y/%m/%d}", | ||
| "url": campsite.booking_url, | ||
| "color": 2375436, | ||
| "fields": [ | ||
| { | ||
| "name": key, | ||
| "value": str(value) | ||
| } for key, value in formatted_dict.items() | ||
| ], | ||
| "footer": { | ||
| "text": "camply, the campsite finder ⛺️" | ||
| } | ||
| } |
There was a problem hiding this comment.
A couple notes on this - I think there are a good number of fields that we want to keep from our formatted_dict here - although it can be quite verbose when looking at the notifications the extra context can be useful.
- This
titlefield here assumes that the campsite has a numericcampsite_loop_name- but in a good number of campsites this is a string value. - There are provider's who don't allow the passing of campsite ID via the booking URL - for those cases we'll need to keep the campsite ID in the message
- Is it possible to have a notification open in a "collapsed" view that can be expanded? This would be nice to have the default view be simple, but with expanded details.
See here for the test-notifications CLI command:
camply test-notifications --notifications discord
Here's the same notification, that instead uses message_title and doesn't remove all the fields - I think some of these can be omitted though like Booking Date / Booking URL which are provided elsewhere
| def __init__(self): | ||
| super().__init__() | ||
| self.session.headers.update({"Content-Type": "application/json"}) | ||
| if any([DiscordConfig.DISCORD_WEBHOOK is None, DiscordConfig.DISCORD_WEBHOOK == ""]): |
There was a problem hiding this comment.
There are a couple lines like this that haven't been formatted by black / ruff - that's why we're seeing linting errors.
pre-commit resolves this with hatch - so either of these commands will format the codebase for you:
hatch run format
pre-commit run --all-files
This part is kind of confusing. CI/CD runs both linting (black/ruff) and type checking (mypy) on the codebase inside the Lint job - but only linting errors actually cause it to fail. type-checking issues (on related and unrelated code) will still show up as annotations on the PR but won't actually cause the Lint job to fail.
|
Any updates here? The discord integration would be useful. Happy to pick this up & run, but def don't want to step on any feet! |
|
I haven't spent any time on this, I can take a closer look soon though. In the meantime Discord notifications are supported via the AppRise notification provider https://juftin.com/camply/command_line_usage/#send-a-notification-using-apprise-compatible-services |
|
This was my first time using Apprise, but it was easier to setup than I thought. There may be some additional behavior that this PR adds, but I haven't read it in depth at this point. For future readers, create a webhook on your discord server, copy the URL and set APPRISE_URL with that value (either with |
3 participants
Description
Adds a notification provider for Discord. The user needs to provide a webhook URL similar to Slack.
Has This Been Tested?
Yes
Checklist:
.pre_commiton all my codeThe linter returns some errors on code I didn't change in this PR, ignored those.