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) Send notifications instead of chat announce for posts at staff area #1197

Merged
merged 1 commit into from
Mar 5, 2020
Merged

(Add) Send notifications instead of chat announce for posts at staff area #1197

merged 1 commit into from
Mar 5, 2020

Conversation

shlandturtle
Copy link
Contributor

Select whether to send notifications to staff members instead of announcing posts in chatbox for a given forum area, determined by ID. In future could/should be improved by having a no_announce flag for forum areas for example.

@codecov
Copy link

codecov bot commented Mar 3, 2020

Codecov Report

Merging #1197 into master will increase coverage by 3.48%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1197      +/-   ##
============================================
+ Coverage     23.35%   26.84%   +3.48%     
- Complexity     4140     4152      +12     
============================================
  Files           296      296              
  Lines         13109    13137      +28     
============================================
+ Hits           3062     3527     +465     
+ Misses        10047     9610     -437
Impacted Files Coverage Δ Complexity Δ
app/Notifications/NewTopic.php 0% <0%> (ø) 4 <0> (+1) ⬆️
app/Notifications/NewPost.php 0% <0%> (ø) 5 <0> (+1) ⬆️
app/Http/Controllers/PostController.php 0% <0%> (ø) 18 <0> (+3) ⬆️
app/Http/Controllers/TopicController.php 0% <0%> (ø) 23 <0> (+3) ⬆️
app/Models/Forum.php 0% <0%> (ø) 21 <2> (+2) ⬆️
app/Models/Topic.php 21.42% <0%> (-5.05%) 16 <2> (+2)
app/Http/Controllers/AnnounceController.php 46.3% <0%> (ø) 65% <0%> (ø) ⬇️
app/Models/User.php 37.69% <0%> (+1.19%) 122% <0%> (ø) ⬇️
app/Services/Data/Movie.php 33.33% <0%> (+4.76%) 40% <0%> (ø) ⬇️
app/Http/Controllers/API/TorrentController.php 73.63% <0%> (+5.47%) 83% <0%> (ø) ⬇️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b09be3...e3c6976. Read the comment docs.

Copy link
Contributor

@cbj4074 cbj4074 left a comment

Choose a reason for hiding this comment

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

@HDVinnie I've reviewed this and I don't see any issues with the code (aside from the super-nitpicky style change regarding string concatenation and using sprintf() over the . operator).

Do you have any concerns with the business logic or addition of the feature itself?

@HDVinnie
Copy link
Collaborator

HDVinnie commented Mar 5, 2020

@HDVinnie I've reviewed this and I don't see any issues with the code (aside from the super-nitpicky style change regarding string concatenation and using sprintf() over the . operator).

Do you have any concerns with the business logic or addition of the feature itself?

Im fine with it. Just needs the styleci action to pass before merging.

@shlandturtle
Copy link
Contributor Author

Removed the extra line StyleCI complained about. I do not understand what the codecov error is about.

@cbj4074
Copy link
Contributor

cbj4074 commented Mar 5, 2020

Thanks! Don't worry about the Codecov error.

@HDVinnie HDVinnie merged commit ce483b1 into HDInnovations:master Mar 5, 2020
@shlandturtle shlandturtle deleted the staff-forum-area branch March 8, 2020 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants