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

Add notifications #4

Merged
merged 5 commits into from Feb 5, 2020
Merged

Add notifications #4

merged 5 commits into from Feb 5, 2020

Conversation

@imorland
Copy link
Contributor

imorland commented Feb 5, 2020

This PR adds a new notification (alert and email, default is alert only) to let a user know that their post has been set as a best answer. Configurable in user settings.

image

image

Also adds an email blade for the existing 'reminder' notification

…l notification for 'reminder' notification
@imorland imorland requested a review from datitisev Feb 5, 2020
imorland added 3 commits Feb 5, 2020
@luceos

This comment has been minimized.

Copy link
Member

luceos commented Feb 5, 2020

Great job Ian. Can't do a checkout right now, but love the effort that went into this.

Copy link
Member

datitisev left a comment

Overall looks good, commented on a few nitpicky things.

Any reason why the views are loaded with a whole service provider instead of just using the view factory in extend.php? Also - views should go in resources and not assets because the assets folder is copied by Flarum into the public /assets/extensions directory.

$views->addNamespace('fof-best-answer', __DIR__.'/resources/views');
@imorland

This comment has been minimized.

Copy link
Contributor Author

imorland commented Feb 5, 2020

I'll move from the assets to resources as you suggest. That means I should probably do the same on fof/byobu too, as I added the view there too - https://github.com/FriendsOfFlarum/byobu/blob/master/assets/views/emails/privateDiscussionCreated.blade.php

Not tried using the view factory to date, will give it a try now, thanks for the tip!

@imorland imorland requested a review from datitisev Feb 5, 2020
@datitisev

This comment has been minimized.

Copy link
Member

datitisev commented Feb 5, 2020

👍. Did you test that the Post::find code works? If not, I can do so - I assume it works but just in case.

@imorland

This comment has been minimized.

Copy link
Contributor Author

imorland commented Feb 5, 2020

Yes, works perfectly

@datitisev

This comment has been minimized.

Copy link
Member

datitisev commented Feb 5, 2020

Great. Thanks!

@datitisev datitisev merged commit e1c918c into FriendsOfFlarum:master Feb 5, 2020
2 checks passed
2 checks passed
WIP Ready for review
Details
continuous-integration/styleci/pr The analysis has passed
Details
@imorland

This comment has been minimized.

Copy link
Contributor Author

imorland commented Feb 5, 2020

Thanks for merging, a few customization options coming up next, most likely will be ready tomorrow 🤞🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.