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.

Sign up for GitHub

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

Jump to bottom

Add in-app notification configuration #8375

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open

Add in-app notification configuration #8375

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4be89fc
Add schema changes to user preferences to support in notifications
cmrd-senya Jul 21, 2022
048b204
Update code to use email_enabled attribute instead of record presence
cmrd-senya Jul 21, 2022
e55b876
Update specs for #update_user_preferences
cmrd-senya Jul 23, 2022
8abf74c
Use setting for in app notification disabled
cmrd-senya Jul 23, 2022
8e221db
Add frontend for in app notification settings
cmrd-senya Jul 23, 2022
0b0e146
Notification settings: add header
cmrd-senya Jul 23, 2022
1c4a897
Notification settings: update specs
cmrd-senya Jul 23, 2022
dede976
Notification settings: fix cucumber tests
cmrd-senya Jul 23, 2022
04dcbf6
Remove debug comment
cmrd-senya Jul 23, 2022
790117c
Merge branch 'develop' into 7686-db-schema-update
cmrd-senya Jul 31, 2022
f92775b
Notifications: move notify methods to services from models
cmrd-senya Aug 18, 2022
79031b3
Merge remote-tracking branch 'origin/develop' into 7686-db-schema-update
cmrd-senya Aug 18, 2022
80759b7
Notifications: make email sending independend from notification model
cmrd-senya Aug 18, 2022
90678ab
Move mail method from User model to NotificationService
cmrd-senya Aug 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion spec/controllers/users_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,17 +250,33 @@
context "for #{email_type}" do
it "lets the user turn off mail" do
par = {id: @user.id, user: {email_preferences: {email_type => "true"}}}

expect {
put :update, params: par
}.to change(@user.user_preferences, :count).by(1)
Comment on lines 264 to 266
Copy link
Member

Choose a reason for hiding this comment

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

I think this expect isn't needed anymore, the new check below is enough as it represents the new behaviour.

Suggested change
expect {
put :update, params: par
}.to change(@user.user_preferences, :count).by(1)
put :update, params: par

The same goes for lets the user get mail again below


expect(
@user.user_preferences.find_by(email_type: email_type)
).to have_attributes(
email_enabled: false,
in_app_enabled: true
)
end

it "lets the user get mail again" do
@user.user_preferences.create(email_type: email_type)
par = {id: @user.id, user: {email_preferences: {email_type => "false"}}}

expect {
put :update, params: par
}.to change(@user.user_preferences, :count).by(-1)
}.to change(@user.user_preferences, :count).by(0)

expect(
@user.user_preferences.find_by(email_type: email_type)
).to have_attributes(
email_enabled: true,
in_app_enabled: true
)
end
end
end
Expand Down
35 changes: 33 additions & 2 deletions spec/models/user_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,37 @@
@pref_count = UserPreference::VALID_EMAIL_TYPES.count
end

it "creates records for notification preferences" do
expect {
alice.update_user_preferences(
"mentioned" => "false",
"contacts_birthday" => "false",
"private_message" => "true"
)
}.to change(alice.user_preferences, :count).by(3)

expect(
alice.user_preferences.find_by(email_type: "mentioned")
).to have_attributes(
email_enabled: true,
in_app_enabled: true
)

expect(
alice.user_preferences.find_by(email_type: "contacts_birthday")
).to have_attributes(
email_enabled: true,
in_app_enabled: true
)

expect(
alice.user_preferences.find_by(email_type: "private_message")
).to have_attributes(
email_enabled: false,
in_app_enabled: true
)
end

it "unsets disable mail and makes the right amount of prefs" do
alice.disable_mail = true
expect {
Expand All @@ -459,8 +490,8 @@
it "still sets new prefs to false on update" do
alice.disable_mail = true
expect {
alice.update_user_preferences({"mentioned" => false})
}.to change(alice.user_preferences, :count).by(@pref_count - 1)
alice.update_user_preferences({"mentioned" => "false"})
}.to change(alice.user_preferences, :count).by(@pref_count)
Comment on lines 501 to +503
Copy link
Member

Choose a reason for hiding this comment

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

Instead of checking the rows count, a check for if the correct attributes were set would probably be more useful.

Also this should set settings for both email and in_app.

expect(alice.reload.disable_mail).to be false
end
end
Expand Down