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

feat(notification): add Pushover #4543

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

zaosoula
Copy link
Contributor

@zaosoula zaosoula commented Dec 11, 2024

Changes

  • Add the necessary change to allow user to configure Pushover notification channel
    IMG_7235

@zaosoula
Copy link
Contributor Author

zaosoula commented Dec 11, 2024

cc @peaklabs-dev.
Still need to complete the checklist but here is the updated version of the PR. I restarted from scratch following the changes in #4525

@peaklabs-dev peaklabs-dev added the 🛠️ Feature Issues requesting a new feature. label Dec 11, 2024
@zaosoula
Copy link
Contributor Author

zaosoula commented Dec 11, 2024

I'm stuck with this error, when opening /notification/pushover

Cannot assign Illuminate\Database\Eloquent\Relations\HasOne to property App\Livewire\Notifications\Pushover::$settings of type App\Models\PushoverNotificationSettings
Screenshot 2024-12-11 at 16 46 08

@zaosoula
Copy link
Contributor Author

@peaklabs-dev To test it you, can follow this guide (random link found on google, but the config steps are similar) https://www.cybercom-software.com/help/phonepadadmin/pushover2.htm

Then you can receive notification on your devices

@peaklabs-dev
Copy link
Member

I will check

{
try {
$this->team = auth()->user()->currentTeam();
$this->settings = $this->team->pushoverNotificationSettings();
Copy link
Member

Choose a reason for hiding this comment

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

This line can not have (). it needs to be:
$this->settings = $this->team->pushoverNotificationSettings;

Copy link
Member

Choose a reason for hiding this comment

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

@zaosoula Found it (at least this should be the issue).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect thank you
I will proceed with the testing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's working
Thanks for the help
IMG_7234

@peaklabs-dev
Copy link
Member

Thank you. So is it ready to be merged?
Can you please open a Docs PR with an easy to follow step by step guide on how to set it up?

@peaklabs-dev
Copy link
Member

Also I do not understand what to use for the user I need to use my user key right?
And the token is an API token from an application correct?

@zaosoula
Copy link
Contributor Author

@peaklabs-dev The user and token values got inverted by an error in my code it fixed now.

@peaklabs-dev
Copy link
Member

Give me a minute to test if it works, If I can get it to work I will do the docs for you.

@peaklabs-dev peaklabs-dev marked this pull request as ready for review December 11, 2024 17:40
@peaklabs-dev
Copy link
Member

Thank you for the PR and your fast fixing of the issues. I will add a few small cosmetic fixes.

@peaklabs-dev peaklabs-dev merged commit 86512bb into coollabsio:next Dec 11, 2024
1 check passed
@github-actions github-actions bot removed the 🛠️ Feature Issues requesting a new feature. label Dec 11, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants