Add in-app notification configuration #8375

cmrd-senya · 2022-07-21T19:32:03Z

This PR implements in-app notification configuration feature - ability to turn off in-app notifications per event similar to email. See the comments for screenshots.

Original comment (outdated)

This PR is rather trivial... I don't know if we are okay with moving things in tiny steps. Tell me if we're not.

Our database schema for user notification preferences needs to be extended if we want to support another type of notifications, i.e. in app notifications.

Currently the presence of the user preference record means that the email notification is disabled while absence means it is enabled.

If we want to add in-app notifications and if we want to make them configurable independently from email notifications then we need to support 4 states, not just 2 like now.

Therefore I added 2 new columns -- email_enabled and in_app_enabled.

The default states of the columns correspond current behavior. This is done for backward compatibility. In app notifications are enabled because they are always enabled currently.

Email notifications are disabled because the presence of the record means they are disabled.

Having null: false and the default value set will "back fill" the existing records to correct values in the databases during the migration or lazily depending on the DB type and version.

In the next step (which I imagine to be a separate PR) we can update the existing code to check for email_enabled field value instead of the presence of the record. This will prepare us for the scenarios where the record exists but email_enabled is true.

We can also rename email_type column to notification_type sometime later.

db/migrate/20220721190153_add_email_enabled_to_user_preferences.rb

SuperTux88

Looks good to me so far. But I'm not really sure if it's worth already merging single commits that don't do anything useful yet (well, they do something useful for the future, but merging them now doesn't have any benefit). It only fills the commit log with merge commits and later makes it more difficult to find connected commits (so other steps for the same feature, when they were spread out as single merges).

What I'm absolutely fine with (or even prefer) is, doing small steps in a PR, so having smaller commits with a clear commit message, to split a branch into multiple steps, that makes the reviewing a lot easier. And you are also welcome to ask for feedback or a review whenever you want, even when the feature isn't completely done yet.

There are some scenarios when it's also worth to already merge a PR before the feature is completely done, for example:

when it already does something useful for the user, so users can profit from it and don't need to wait for the full feature
when there are some cases that would be useful to have them merged so another feature can also build on the changes
or when the PR becomes bigger and there is risk of having conflicts with other branches (so similar to above, if other features are working in the same area)
or maybe something else that doesn't come to my mind at the moment, this list isn't complete

But none of this is the case for this PR yet, so I would just continue pushing your next steps to this PR and ask for feedback whenever you want.

Small question regarding your plan for the next steps: are you planning to insert a true/true entry for every notification type and user, or do you want to still only inserting stuff when the user changes the defaults, but assume true/true if there is no entry?

cmrd-senya · 2022-07-21T20:51:11Z

Small question regarding your plan for the next steps: are you planning to insert a true/true entry for every notification type and user, or do you want to still only inserting stuff when the user changes the defaults, but assume true/true if there is no entry?

I was thinking about the latter - assuming true/true when there is no entry

SuperTux88 · 2022-07-21T20:55:26Z

Yes, I also prefer it that way I think. Keeping the table full of true/true for all users would be more complicated (we would also need to insert new entries every time we add a new notification type, for all users), and I think using the table only to store the state for users who changed away from the default makes more sense. And it's also how the table is used at the moment, because we already have entries for all users who changed away from the default 👍

app/models/user.rb

cmrd-senya · 2022-07-23T18:15:48Z

Okay, we're almost there. It's left to update the specs.

SuperTux88 · 2022-07-23T18:24:23Z

Maybe add an explanation or a header at the top of the two columns to explain what they mean ... because at the moment somebody could also understand it as they receive a private message instead of a mail? 🤔

cmrd-senya · 2022-07-23T18:25:57Z

Maybe add an explanation or a header at the top of the two columns to explain what they mean ... because at the moment somebody could also understand it as they receive a private message instead of a mail? thinking

Yes, I thought about that too. I can add a header. Maybe also some kind of tooltip could be helpful?

SuperTux88 · 2022-07-23T18:27:34Z

Yes, a tooltip would also work, but tooltips only work on desktop, not on mobile with touch as you can't hover over things.

cmrd-senya · 2022-07-23T20:51:09Z

app/views/users/_edit.haml

@@ -122,61 +122,180 @@
      .row
        .col-md-12
          %h3
-            = t(".receive_email_notifications")


Should we clean up existing translations for this string? Or it will be done automatically when we sync with the translation platform?

You only need to update the English translations, other languages are managed by webtranslateit.

cmrd-senya · 2022-07-23T21:08:37Z

Added hearer

cmrd-senya · 2022-07-23T23:51:00Z

@SuperTux88 I think it can be reviewed now. Let me know if I should add some additional specs.

SuperTux88

As I also mentioned in some comments, but TL;DR: I would also refactor how this whole thing works some more and also change some naming, as everything is touched now anyway, so I think that's a good moment to clean some stuff up that was already confusing before and now is even more confusing.

rename user_preferences/email_preferences to notification_settings
also rename/remove some mentions of email in variables where it now also means in-app notifications
fix that the booleans are the wrong way around when submitting the form, true should always mean enabled, false should always mean disabled.

Again, most of this isn't because of stuff you changed now, it was already confusing in the past, I think it's just a good opportunity to make it less confusing as most of the code is touched now anyway.

Also, the way you implemented it now, it doesn't send mails anymore if you disable in-app notifications. But emails should still be sent if they are enabled. So you also need to refactor how this works, so you are able to send emails without persisting an in-app notification to the database.

But overall I think this goes into the right direction 👍

SuperTux88 · 2022-07-24T01:27:48Z

app/models/user.rb

+      UserPreference::VALID_EMAIL_TYPES.each do |type|
+        user_preferences.find_or_create_by(email_type: type).update(email_enabled: false)
+      end


I don't even know why this is here (looks like something very old), and it doesn't make a lot of sense to me. Maybe 10 years ago you could only disable all mails or enable all mails, and this was some sort of migration code ... but nothing in the UI reads the disable_mail flag, so setting all types to disabled silently in the backend when changing the settings in the frontend doesn't make a lot of sense. We should just set what the user did in the frontend, so I would just drop these 3 lines.

Suggested change

UserPreference::VALID_EMAIL_TYPES.each do |type|

user_preferences.find_or_create_by(email_type: type).update(email_enabled: false)

end

Yes, it seemed to me as well that it is not being used. I'll remove it.

SuperTux88 · 2022-07-24T01:37:24Z

app/models/user.rb

+    email_enabled = (disable_mail == false) &&
+      NotificationSettingsService.new(self).email_enabled?(pref)
+
+    job.perform_async(*args) if email_enabled


I would probably write this as guard clause:

Suggested change

email_enabled = (disable_mail == false) &&

NotificationSettingsService.new(self).email_enabled?(pref)

job.perform_async(*args) if email_enabled

return if disable_mail || NotificationSettingsService.new(self).email_disabled?(pref)

job.perform_async(*args)

But either way, (disable_mail == false) doesn't make much sense, just use !disable_mail

SuperTux88 · 2022-07-24T01:40:33Z

app/services/notification_settings_service.rb

+
+  attr_reader :user
+
+  delegate :user_preferences, to: :user


user_preferences is just used once here ... so I would just use user.user_preferences that one time instead.

SuperTux88 · 2022-07-24T01:44:08Z

spec/controllers/users_controller_spec.rb

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


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

SuperTux88 · 2022-07-24T01:50:21Z

spec/models/user_spec.rb

      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)


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.

SuperTux88 · 2022-07-24T02:27:51Z

db/migrate/20220721190153_add_email_enabled_to_user_preferences.rb

+    change_table :user_preferences, bulk: true do |t|
+      t.boolean :email_enabled, null: false, default: false
+      t.boolean :in_app_enabled, null: false, default: true
+    end


I would like to suggest some general changes regarding this whole thing, as this is touched now anyways. To me the naming of this table never made sense, as it's specific a table for notifications and not about "user preferences" ... so I would suggest renaming it to something that explains what this is, for example notification_settings (as you named the service NotificationSettingsService), and then also rename the variables and everything to something regarding notifications. I think that will make the code overall more readable because I was always confused about this.

Also, after thinking about it a bit more, I don't think the _enabled suffix for the new columns is needed, I think it's self-explained anyway, but if you want to keep it, that's fine for me too.

Also, when you are at it, can you rename the existing email_type column to just type, since it's not about emails anymore. I also think that will make this more readable for the future.

SuperTux88 · 2022-07-24T02:32:32Z

app/controllers/users_controller.rb

@@ -137,7 +137,7 @@ def user_params
      :post_default_public,
      :exported_photos_file,
      :export,
-      email_preferences: UserPreference::VALID_EMAIL_TYPES.map(&:to_sym)
+      email_preferences: UserPreference::VALID_EMAIL_TYPES.to_h {|type| [type.to_sym, %i[mail in_app]] }


As this is not about emails only anymore, renaming everything that is now email_preferences (or email_prefs) to notification_settings would make more sense to me.

SuperTux88 · 2022-07-24T02:39:03Z

app/models/user.rb

+      email_enabled = pref_hash[key]["mail"] == "false"
+      in_app_enabled = pref_hash[key]["in_app"] == "false"


Can you please also remove this negation here. It should be true if it's enabled everywhere and false if it's disabled, but at the moment when submitting the form it's exactly the wrong way around and that's extremely confusing. I also already found this negation confusing in the past, but it at least kinda made sense because how the table worked in the past.

SuperTux88 · 2022-07-24T02:47:35Z

spec/models/user_spec.rb

+      expect {
+        alice.update_user_preferences(
+          "mentioned"         => {
+            "mail"   => "false",
+            "in_app" => "false"
+          },
+          "contacts_birthday" => {
+            "mail"   => "false",
+            "in_app" => "false"
+          },
+          "private_message"   => {
+            "mail"   => "true",
+            "in_app" => "false"
+          }
+        )
+      }.to change(alice.user_preferences, :count).by(3)


I don't think the number of changed rows is important here, as the checks below already verify the changes that are really important here, so I would remove the expect block around here.

But maybe test different combinations of email and in_app enabled/disabled.

SuperTux88 · 2022-07-24T02:51:45Z

app/assets/stylesheets/user_preferences.scss

+  }
+
+  [type="checkbox"]:checked ~ i {
+    color: $black;


This needs some override for the dark theme in _color_theme_override_dark.scss for the dark theme ($white?) and maybe also for the color when unchecked.

cmrd-senya · 2022-07-24T08:09:26Z

Thanks for the feedback, indeed, I didn't realize that sending email notifications depends on the Notification record presence. I can see how to fix that but it'll require a bit more refactoring. I'll work on it.

cmrd-senya · 2022-07-24T09:31:00Z

@SuperTux88 I'm thinking about moving these self.notify methods out of the models:

diaspora/app/models/notifications/comment_on_post.rb

Line 15 in e826909

def self.notify(comment, _recipient_user_ids)

I'd like to put the code to the "service" layer, possibly creating a new service object for it. There is already NotificationService but it will be too much to put everything there so I'd rather create another one. Could be something like NotifyUserService or SendNotificationService. I like service names to be more like verbs than like nouns. NotificationService#notify method could also be moved to that new service too. Basically NotificationService is doing a few different things right now which don't need to be kept in one class, I think that a service should implement one action, not a few different like index, create and update at once.

How does it sound? Would it be okay?

SuperTux88 · 2022-07-24T14:35:05Z

Sounds good to me so far. I don't mind too much if one service does multiple things about notifications, or if there are different services for it. But if it otherwise would become too big, it's probably better to split them up so it's clearer to read what belong together and also maybe easier to test. As the current notify method doesn't add a lot to the NotificationService at the moment, so it was fine to have it in there, but if it now gets more logic to differ between mail and in-app, it probably makes sense to split stuff up a bit. But I don't know what exactly is needed yet, but I think you'll figure that out as you go 👍

diaspora-code-review · 2022-08-18T21:00:25Z

app/services/notifications/mentioned_in_comment_service.rb

+      Notifications::MentionedInComment
+    end
+
+    def self.notify(mentionable, recipient_user_ids)


Metrics/AbcSize: Assignment Branch Condition size for notify is too high. [<6, 27, 6> 28.3/20]

cmrd-senya · 2022-08-18T21:30:22Z

@SuperTux88 so after some thought I went with the following solution: I moved all self.notify methods from the notification models to respective services which I put to app/services/notifications/*_service.rb (commit). Then I changed the self.notify methods in the services the way that the email sending code wasn't dependent on existing notification record instance (commit).

How do you like that?

I haven't updated the specs yet, thought better to discuss if the refactoring looks okay.

I'm also thinking of moving User#mail method somewhere to the services from the user model.

diaspora-code-review · 2022-08-20T11:07:18Z

app/services/notifications/also_commented_service.rb

+
+module Notifications
+  class AlsoCommentedService
+    def self.notify(comment, _)


Metrics/AbcSize: Assignment Branch Condition size for notify is too high. [<4, 20, 2> 20.49/20]

diaspora-code-review · 2022-08-20T11:07:19Z

app/services/notifications/mentioned_in_comment_service.rb

+      Notifications::MentionedInComment
+    end
+
+    def self.notify(mentionable, recipient_user_ids)


Metrics/AbcSize: Assignment Branch Condition size for notify is too high. [<6, 28, 6> 29.26/20]

Metrics/MethodLength: Method has too many lines. [25/20]

SuperTux88

I think overall this goes into the right direction, but there are still some issues with handling in-app and mails differently and some places still relying on the notification objects and some other minor comments.

SuperTux88 · 2022-08-21T23:37:39Z

app/services/notification_service.rb

+    email_enabled = (user.disable_mail == false) &&
+      NotificationSettingsService.new(user).email_enabled?(pref)
+
+    job.perform_async(*args) if email_enabled


I still would write this as guard clause and the == false doesn't make a lot of sense (see old comment)

Suggested change

email_enabled = (user.disable_mail == false) &&

NotificationSettingsService.new(user).email_enabled?(pref)

job.perform_async(*args) if email_enabled

return if user.disable_mail || NotificationSettingsService.new(user).email_disabled?(pref)

job.perform_async(*args)

SuperTux88 · 2022-08-21T23:52:45Z

app/services/notification_service.rb

@@ -2,13 +2,17 @@

 class NotificationService
  NOTIFICATION_TYPES = {


Maybe rename that NOTIFICATION_SERVICES now?

SuperTux88 · 2022-08-22T00:06:23Z

app/services/notifications/also_commented_service.rb

+        NotificationService.new(recipient).mail(
+          Workers::Mail::AlsoCommented,
+          recipient.id,
+          actor.id,
+          comment.id
+        )


It's a bit duplicate that the recipient is already a parameter for the service, and then the recipient.id is added again as parameter for the mail method. I think we can remove the recipient.id parameter here for all notification services, and then set it for all in the NotificationService#mail:

job.perform_async(user.id, *args)

SuperTux88 · 2022-08-22T00:25:39Z

app/services/notifications/also_commented_service.rb

+      recipient_ids = commentable.participants.local.where.not(id: [commentable.author_id, actor.id]).pluck(:owner_id)
+
+      User.where(id: recipient_ids).find_each do |recipient|
+        next if recipient.is_shareable_hidden?(commentable) || mention_notification_exists?(comment, recipient.person)


The mention_notification_exists?(comment, recipient.person) still relies on the notification object existing. So if somebody has disabled the in-app notifications for mentions (but enabled mails for both mentions and comments), they would receive two mails for the same comment, as this would trigger a second notification, as the mention notification object wasn't created.

I don't know what's the best solution for this. The mentions notification already handles mails being disabled for mentions, and then sends a normal commented mail. But when we would for example check here, if there is a mention in the comment, then we wouldn't send duplicate mails (but still one mail, as that's already handled in the mentions notification service ... but we wouldn't send an in-app notification if that's disabled for mentions (unless we also move that logic to the mentions notification service as well?) But then we have even more logic in the mentions notification service, that's kinda duplicated? 🤷‍♂️ Maybe you have an idea how to solve that the best?

SuperTux88 · 2022-08-22T00:37:28Z

app/models/notification.rb

@@ -53,6 +45,11 @@ def self.create_notification(recipient, target, actor)
  end

  private_class_method def self.suppress_notification?(recipient, actor)
-    recipient.blocks.where(person: actor).exists?
+    return true if recipient.blocks.exists?(person: actor)


This now isn't called anymore for mails, so it now always sends mails, even if the actor is blocked/ignored. But as you extended this now to also check if in-app notifications are enabled, it can't just be called for mails. So I think the best would be to split this up again, so this can be called both for mails and for in-app notifications, and then only check for in-app notifications enabled/disabled in Notification.create_notification and Notification.concatenate_or_create?

SuperTux88 · 2022-08-22T00:57:13Z

app/services/notifications/private_message_service.rb

+        Notifications::PrivateMessage
+          .new(recipient: recipient)


This isn't needed, this was only a workaround because the mail needed the object, but it's never persisted.

SuperTux88 · 2022-08-22T00:59:48Z

app/services/notifications/mentioned_in_post_service.rb

+    def self.notify(mentionable, recipient_user_ids)
+      actor = mentionable.author
+      relevant_mentions = filter_mentions(
+        mentionable.mentions.local.where.not(person: actor),
+        mentionable,
+        recipient_user_ids
+      )
+
+      relevant_mentions.each do |mention|
+        recipient = mention.person.owner
+        next if model.exists?(recipient: recipient, target: mention)
+
+        model
+          .create_notification(recipient, mention, actor)
+
+        NotificationService.new(recipient).mail(
+          Workers::Mail::Mentioned,
+          recipient.id,
+          actor.id,
+          mention.id
+        )
+      end
+    end


This was used for both mentions in post and mentions in comments before, but the code now got duplicated. Maybe this can be extracted to an abstract mentioned notification service (needs to add a mail_job method again instead of hardcoding the job, but the model is already called from a method).

SuperTux88 · 2022-08-22T01:10:46Z

app/services/notifications/mentioned_in_post_service.rb

+
+      relevant_mentions.each do |mention|
+        recipient = mention.person.owner
+        next if model.exists?(recipient: recipient, target: mention)


This relies on the notification entity existing, which isn't the case if in-app notifications are disabled. This was a workaround for when posts were received twice, no duplicate notifications were created (#7721). I don't remember all the details and reasons, but Diaspora::Federation::Receive.status_message still processes a received post even, if it already exists, and because of that it also calls ReceiveLocal again.

I don't know/remember why a post should be received again if it already exists, maybe that was a workaround for when a post was created incomplete because of the scenario described here where some data for posts was missing, and receiving them again properly through the federation, would add this missing data. But if that gets fixed so it fetches the posts using federation code, it doesn't need to be received a second time, as it was already complete the first time. That would then also solve the problem of duplicate notifications, without needing the check for them here (as I don't know how otherwise this should be solved for not sending duplicate mails).

@SuperTux88 is it enough to do the proper fetching using federation code? It seems quite easy.

Commit: ecb3923

Didn't look at the tests yet.

~~(Probably worth adding exception handling too)~~ Updated

is it enough to do the proper fetching using federation code? It seems quite easy.

Yes, that's theoretically all that's needed, but the type should be the lowercase, snake_case version of the entity name (doc).

But you can even improve that a little bit more by just using :post as type (as it could also be a reshare) and you can remove the check_type in validate, so that way we can even fetch reshares (you could also use the type from the json, but then you would need to convert that to the snake_case version, so just always use :post works and is easier) :)

But yes, basically that, it's fairly easy 👍

And the exception should be DiasporaFederation::Federation::Fetcher::NotFetchable.

cmrd-senya · 2022-08-22T17:49:40Z

Thanks for the detailed review @SuperTux88! Gonna work through this 👍

diaspora-code-review reviewed Jul 21, 2022

View reviewed changes

db/migrate/20220721190153_add_email_enabled_to_user_preferences.rb Show resolved Hide resolved

cmrd-senya force-pushed the 7686-db-schema-update branch from 7588f5e to 4be89fc Compare July 21, 2022 19:38

Add schema changes to user preferences to support in notifications

4be89fc

cmrd-senya marked this pull request as ready for review July 21, 2022 19:54

cmrd-senya requested review from denschub and SuperTux88 July 21, 2022 19:54

SuperTux88 approved these changes Jul 21, 2022

View reviewed changes

cmrd-senya marked this pull request as draft July 21, 2022 21:25

cmrd-senya force-pushed the 7686-db-schema-update branch from 5015dec to 048b204 Compare July 22, 2022 23:07

Update code to use email_enabled attribute instead of record presence

048b204

cmrd-senya commented Jul 22, 2022

View reviewed changes

app/models/user.rb Outdated Show resolved Hide resolved

cmrd-senya added 2 commits July 23, 2022 15:30

Update specs for #update_user_preferences

e55b876

Use setting for in app notification disabled

8abf74c

Add frontend for in app notification settings

8e221db

cmrd-senya commented Jul 23, 2022

View reviewed changes

Notification settings: add header

0b0e146

cmrd-senya changed the title ~~Add schema changes to user preferences to support in notifications~~ Add in-app notification configuration Jul 23, 2022

cmrd-senya force-pushed the 7686-db-schema-update branch from eaa9d49 to dede976 Compare July 23, 2022 23:27

cmrd-senya added 2 commits July 24, 2022 02:29

Notification settings: update specs

1c4a897

Notification settings: fix cucumber tests

dede976

cmrd-senya requested a review from SuperTux88 July 23, 2022 23:48

cmrd-senya marked this pull request as ready for review July 23, 2022 23:48

Remove debug comment

04dcbf6

SuperTux88 requested changes Jul 24, 2022

View reviewed changes

cmrd-senya added 3 commits July 31, 2022 22:05

Merge branch 'develop' into 7686-db-schema-update

790117c

Notifications: move notify methods to services from models

f92775b

Merge remote-tracking branch 'origin/develop' into 7686-db-schema-update

79031b3

diaspora-code-review reviewed Aug 18, 2022

View reviewed changes

Notifications: make email sending independend from notification model

80759b7

diaspora-code-review reviewed Aug 20, 2022

View reviewed changes

Move mail method from User model to NotificationService

90678ab

SuperTux88 requested changes Aug 22, 2022

View reviewed changes

tclaus mentioned this pull request Aug 29, 2022

Re-introduce likes on comments #8203

Merged

denschub removed their request for review September 7, 2022 16:38

	UserPreference::VALID_EMAIL_TYPES.each do \|type\|
	user_preferences.find_or_create_by(email_type: type).update(email_enabled: false)
	end

		email_enabled = pref_hash[key]["mail"] == "false"
		in_app_enabled = pref_hash[key]["in_app"] == "false"

		@@ -2,13 +2,17 @@

		class NotificationService
		NOTIFICATION_TYPES = {

Add in-app notification configuration #8375

Are you sure you want to change the base?

Add in-app notification configuration #8375

Conversation

cmrd-senya commented Jul 21, 2022 • edited

Original comment (outdated)

SuperTux88 left a comment

Choose a reason for hiding this comment

cmrd-senya commented Jul 21, 2022

SuperTux88 commented Jul 21, 2022

cmrd-senya commented Jul 23, 2022

SuperTux88 commented Jul 23, 2022

cmrd-senya commented Jul 23, 2022

SuperTux88 commented Jul 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmrd-senya commented Jul 23, 2022

cmrd-senya commented Jul 23, 2022

SuperTux88 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmrd-senya commented Jul 24, 2022 • edited

cmrd-senya commented Jul 24, 2022

SuperTux88 commented Jul 24, 2022

Choose a reason for hiding this comment

cmrd-senya commented Aug 18, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuperTux88 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmrd-senya Sep 4, 2022 • edited

Choose a reason for hiding this comment

cmrd-senya Sep 4, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmrd-senya commented Aug 22, 2022

cmrd-senya commented Jul 21, 2022 •

edited

cmrd-senya commented Jul 24, 2022 •

edited

cmrd-senya commented Aug 18, 2022 •

edited

cmrd-senya Sep 4, 2022 •

edited

cmrd-senya Sep 4, 2022 •

edited