Improve to manage notifications and proposals #2046

Silver-IT · 2024-06-06T01:24:19Z

Summary of Changes

…s expiring

cypress · 2024-06-06T01:35:53Z

Passing run #2489 ↗︎

0	114	8	0	0

Details:

Merge `07f200c` into `5da85fd`...
Project: group-income	Commit: `4158dfa481 ℹ️`
Status: Passed	Duration: 10:08 💡
Started: Jun 28, 2024 12:34 AM	Ended: Jun 28, 2024 12:44 AM

Review all test suite changes for PR #2046 ↗︎

* PR for release v0.4.2 * feat: save notification body as object format --------- Co-authored-by: Greg Slepak <[email protected]>

Silver-IT

Ready for the review!
Worked on two Issues in this PR, instead of working on separate PRs, in order to reduce conflicts mainly in templates.js file.

Silver-IT · 2024-06-10T12:49:13Z

frontend/model/contracts/group.js

+      validate: actionRequireActiveMember(objectOf({
+        proposalIds: arrayOf(string)
+      })),


To make data meaningful, I've updated the type from array to object.

Silver-IT · 2024-06-10T12:50:56Z

frontend/model/contracts/group.js

@@ -1948,13 +1976,15 @@ sbp('chelonia/defineContract', {
        console.warn(`removeForeignKeys: ${e.name} error thrown:`, e)
      })
    },
-    'gi.contracts/group/emitNotificationAfterSyncing': async (contractIDs, notificationName, notificationData) => {
+    'gi.contracts/group/emitNotificationsAfterSyncing': async (contractIDs, notifications) => {


Updated function to be able to emit multiple notifications.

Silver-IT · 2024-06-10T13:05:23Z

frontend/model/contracts/shared/constants.js

@@ -52,7 +52,6 @@ export const CHATROOM_MEMBER_MENTION_SPECIAL_CHAR = '@'
 export const CHATROOM_CHANNEL_MENTION_SPECIAL_CHAR = '#'

 // chatroom events
-export const CHATROOM_MESSAGE_ACTION = 'chatroom-message-action'


We don't use this action anymore unless we use the contracts of old versions. I remember this action was removed in the PR #1521 where @corrideat implemented E2E Protocol.

Silver-IT · 2024-06-10T13:09:36Z

frontend/model/contracts/shared/functions.js

-export function swapMentionIDForDisplayname (text: string): string {
+export function swapMentionIDForDisplayname (
+  text: string,
+  options: Object = { escaped: true, forChat: true }


Added options argument and made this function more dynamic.

escaped: this indicates that the text contains escaped characters forChat: this indicates that the function is being used for messages inside chatroom

Please add some inline comments to the code to explain what these parameters are along with some examples of which situations you would want to use one versus the other.

Silver-IT · 2024-06-10T13:11:02Z

frontend/model/notifications/selectors.js

      ...template,
+      avatarUserID: template.avatarUserID || sbp('state/vuex/getters').ourIdentityContractId,


Updated order so that avatarUserID could be overridden.

Silver-IT · 2024-06-10T13:12:09Z

historical/views/containers/chatroom/MessageInteractive.vue

@@ -0,0 +1,205 @@
+<template lang='pug'>


Saved this file inside history folder, because I am not sure we could use this file or content later.

historical/views/containers/chatroom/MessageInteractive.vue

taoeffect

Great work @Silver-IT!

taoeffect · 2024-06-18T17:20:39Z

frontend/model/contracts/shared/functions.js

+  return text.split(regEx).map(t => {
+    return possibleMentions.includes(t)
+      ? t[0] === CHATROOM_MEMBER_MENTION_SPECIAL_CHAR
+        ? forChat ? t[0] + usernameFromID(t.slice(1)) : userDisplayNameFromID(t.slice(1))
+        : (forChat ? t[0] : '') + getChatroomNameById(t.slice(1))
+      : t
+  }).join('')


This is written in a far more confusing manner now. Can you please add some parenthesis around the nested ?:'s to make it clearer?

taoeffect · 2024-06-18T17:21:11Z

frontend/model/contracts/shared/functions.js

-export function swapMentionIDForDisplayname (text: string): string {
+export function swapMentionIDForDisplayname (
+  text: string,
+  options: Object = { escaped: true, forChat: true }


Please add some inline comments to the code to explain what these parameters are along with some examples of which situations you would want to use one versus the other.

taoeffect · 2024-06-18T17:26:34Z

frontend/model/notifications/templates.js

+
    return {
-      body: L("{errName} during {activity} for '{action}' from {b_}{who}{_b} to '{contract}': '{errMsg}'", {
-        ...LTags('b'),
-        errName: error.name,
-        activity,
-        action: action ?? opType,
-        who: meta?.username ?? message.signingKeyId(),
-        contract: contractName(contractID),
-        errMsg: error.message ?? '?'
-      }),
+      body: {
+        key: "{errName} during {activity} for '{action}' from {b_}{who}{_b} to '{contract}': '{errMsg}'",
+        args: {
+          ...LTags('b'),
+          errName: error.name,
+          activity,
+          action: action ?? opType,
+          who: `${CHATROOM_MEMBER_MENTION_SPECIAL_CHAR}${message.signingKeyId()}`,
+          contract: sbp('state/vuex/state').contracts[contractID]?.type ?? contractID,
+          errMsg: error.message ?? '?'
+        }
+      },


All of the changes in this file must be reverted because user-facing strings must be wrapped in L functions or the strings utility will not find them.

taoeffect · 2024-06-18T17:26:54Z

frontend/model/notifications/templates.js

+    const args = {
+      name: `${CHATROOM_MEMBER_MENTION_SPECIAL_CHAR}${data.creatorID}`,
+      ...LTags('strong')
+    }
    const bodyTemplateMap = {
-      ADD_MEMBER: () => L('{name} proposed to add a member to the group. Vote now!', { name }),
-      CHANGE_MINCOME: () => L('{name} proposed to change the group mincome. Vote now!', { name }),
-      CHANGE_DISTRIBUTION_DATE: () => L('{name} proposed to change the group distribution date. Vote now!', { name }),
-      CHANGE_VOTING_RULE: () => L('{name} proposed to change the group voting system. Vote now!', { name }),
-      REMOVE_MEMBER: () => L('{name} proposed to remove a member from the group. Vote now!', { name }),
-      GENERIC: () => L('{name} created a proposal. Vote now!', { name })
+      ADD_MEMBER: () => ({ key: '{strong_}{name}{_strong} proposed to add a member to the group. Vote now!', args }),
+      CHANGE_MINCOME: () => ({ key: '{strong_}{name}{_strong} proposed to change the group mincome. Vote now!', args }),
+      CHANGE_DISTRIBUTION_DATE: () => ({ key: '{strong_}{name}{_strong} proposed to change the group distribution date. Vote now!', args }),
+      CHANGE_VOTING_RULE: () => ({ key: '{strong_}{name}{_strong} proposed to change the group voting system. Vote now!', args }),
+      REMOVE_MEMBER: () => ({ key: '{strong_}{name}{_strong} proposed to remove a member from the group. Vote now!', args }),
+      GENERIC: () => ({ key: '{strong_}{name}{_strong} created a proposal. Vote now!', args })


All of the changes in this file must be reverted because user-facing strings must be wrapped in L functions or the strings utility will not find them.

taoeffect · 2024-06-18T17:27:04Z

frontend/model/notifications/templates.js

+    const makeBodyBasedOnCreator = (key) => {
+      return isMe
+        ? { key: '{strong_}Your{_strong} ' + key, args }
+        : {
+            key: "{strong_}{name}'s{_strong} " + key,
+            args: { ...args, name: `${CHATROOM_MEMBER_MENTION_SPECIAL_CHAR}${creatorID}` }
+          }
+    }
    const bodyTemplateMap = {
      [PROPOSAL_INVITE_MEMBER]:
-        (opts) => L('{creator} proposal to add {member} to the group was {closedWith}.', opts),
+        () => makeBodyBasedOnCreator('proposal to add {member} to the group was {strong_}{closedWith}{_strong}.'),
      [PROPOSAL_REMOVE_MEMBER]:
-        (opts) => L('{creator} proposal to remove {member} from the group was {closedWith}.', opts),
+        () => makeBodyBasedOnCreator('proposal to remove {member} from the group was {strong_}{closedWith}{_strong}.'),
      [PROPOSAL_GROUP_SETTING_CHANGE]:
-        (opts) => L('{creator} proposal to change group\'s {setting} to {value} was {closedWith}.', opts),
+        () => makeBodyBasedOnCreator("proposal to change group's {setting} to {value} was {strong_}{closedWith}{_strong}."),
      [PROPOSAL_PROPOSAL_SETTING_CHANGE]:
-        (opts) => L('{creator} proposal to change group\'s {setting} was {closedWith}.', opts), // TODO: define message
+        () => makeBodyBasedOnCreator("proposal to change group's {setting} was {strong_}{closedWith}{_strong}."),
      [PROPOSAL_GENERIC]:
-        (opts) => L('{creator} proposal "{title}" was {closedWith}.', opts)
-    }
-    const statusMap = {
-      [STATUS_PASSED]: { icon: 'check', level: 'success', closedWith: L('accepted') },
-      [STATUS_FAILED]: { icon: 'times', level: 'danger', closedWith: L('rejected') },
-      [STATUS_CANCELLED]: { icon: 'times', level: 'danger', closedWith: L('cancelled') }, // TODO: define icon, level
-      [STATUS_EXPIRED]: { icon: 'times', level: 'danger', closedWith: L('expired') } // TODO: define icon, level
+        () => makeBodyBasedOnCreator('proposal "{title}" was {strong_}{closedWith}{_strong}.')


All of the changes in this file must be reverted because user-facing strings must be wrapped in L functions or the strings utility will not find them.

taoeffect · 2024-06-18T17:27:17Z

frontend/model/notifications/templates.js

+      'pledger': () => ({
+        key: 'A new distribution period ({period}) has started. Please check Payment TODOs.',
+        args
+      }),
+      'receiver': () => ({
+        key: 'A new distribution period ({period}) has started. Please update your income details if they have changed.',
+        args
+      })
    }

    return {
      avatarUserID: data.creatorID,
-      body: bodyTemplate[data.memberType],
+      body: bodyTemplate[data.memberType](),


All of the changes in this file must be reverted because user-facing strings must be wrapped in L functions or the strings utility will not find them.

taoeffect · 2024-06-18T17:58:40Z

frontend/views/containers/chatroom/MessageInteractive.vue

-    .c-icon(
-      :class='{"is-warning": isYellowHorn}'
-    )
+    .c-icon(:class='{"is-warning": isYellowHorn}')
      svg-yellow-horn(v-if='isYellowHorn')
      svg-horn(v-else)
  template(#header='')
    .c-header
-      span.c-title.is-title-5(:class='interactiveMessage.proposalSeverity') {{interactiveMessage.proposalStatus}}
-      span.has-text-1 {{ humanDate(datetime, { hour: 'numeric', minute: 'numeric' }) }}
+      span TODO
  template(#body='')
-    .c-text
-      | {{interactiveMessage.text}}
-      i18n.c-link(@click='$router.push({ path: "/dashboard#proposals" })') See proposal
+    .c-text TODO
 </template>

 <script>
-import { mapGetters } from 'vuex'
-import { L } from '@common/common.js'
-import {
-  PROPOSAL_GROUP_SETTING_CHANGE,
-  PROPOSAL_INVITE_MEMBER,
-  PROPOSAL_REMOVE_MEMBER,
-  PROPOSAL_PROPOSAL_SETTING_CHANGE,
-  PROPOSAL_GENERIC,
-  PROPOSAL_VARIANTS
-} from '@model/contracts/shared/constants.js'
-import { getProposalDetails } from '@model/contracts/shared/functions.js'


Hmm, I'm not sure about this. I think I made a big mistake here when I said "go for it", not fully realizing the implications of my answer, that it would result in a lot of code getting deleted.

What we had here seems very useful. We might change our mind and decide that it's still important to notify users in the #general chatroom about proposals expiring, or about other situations in the group, in order to keep the engagement in the group high.

I'm sorry to ask this of you, as it is my fault for giving you a bad answer earlier, but can we keep all of the code that was here before, but update it so that it uses the STATUS_* variables instead?

I think we should still post these interactive messages to the group chat, so that people who aren't paying attention to the notifications can still be reminded to vote on the proposals.

Silver-IT

Ready for the another review!

taoeffect

Really great PR @Silver-IT! I tested it and it seems to work fairly well, but I wasn't able to test everything.

For example, I wasn't able to test expiring proposals. Is there an easy way to test that?

I also noticed that chatroom messages for when proposals are created or expiring or closed are not posted to the #general chatroom. Can they be posted? It seems like something that would be useful.

frontend/model/notifications/templates.js

taoeffect · 2024-06-27T16:59:41Z

frontend/model/notifications/templates.js

      scope: 'group'
    }
  },
  MEMBER_LEFT (data: { groupID: string, memberID: string }) {
-    const name = strong(userDisplayNameFromID(data.memberID))


This function seemed useful to DRY things, why get rid of it?

taoeffect · 2024-06-27T17:51:37Z

frontend/model/notifications/templates.js

    return {
      body: L("{errName} during {activity} for '{action}' from {b_}{who}{_b} to '{contract}': '{errMsg}'", {
        ...LTags('b'),
        errName: error.name,
        activity,
        action: action ?? opType,
-        who: meta?.username ?? message.signingKeyId(),
-        contract: contractName(contractID),
+        who: `${CHATROOM_MEMBER_MENTION_SPECIAL_CHAR}${message.signingKeyId()}`,


Please note: the message.signingKeyId() is NOT a contractID. So you cannot use it to get the username and therefore CHATROOM_MEMBER_MENTION_SPECIAL_CHAR is also inappropriate here.

In fact it seems there's currently no simple way to figure out this information from a GIMessage, and not all GIMessages contain this information.

So I've created #2125 to address this and assigned it to @corrideat, but for now let's just remove this who variable altogether.

Silver-IT · 2024-06-27T22:46:28Z

I wasn't able to test expiring proposals. Is there an easy way to test that?

To be honest, I didn't test the expiring proposals for this PR, because they are already tested before and I didn't make any changes related it.
But I remember I was testing it using manual update of system clock. 😁🙈

I also noticed that chatroom messages for when proposals are created or expiring or closed are not posted to the #general chatroom. Can they be posted? It seems like something that would be useful.

Sure, why not? I will make it work then.

taoeffect

Additional minor change request here

taoeffect · 2024-06-28T17:59:43Z

frontend/model/contracts/shared/types.js

  proposal: objectMaybeOf({
    proposalId: string,
    proposalType: string,
    expires_date_ms: number,
    createdDate: string,
    creatorID: string,
-    variant: unionOf(...Object.values(PROPOSAL_VARIANTS).map(v => literalOf(v)))
+    variant: unionOf([STATUS_EXPIRING].map(v => literalOf(v))) // NOTE: only expiring proposals could be notified at the moment


@Silver-IT Please feel free to add all of the types of proposals here, as we want to be able to notify the #general chatroom of anything related to proposals (being opened, closed, etc.).

I was planning to add all types while I would be working on Issue #2138 after this PR is merged. Do you want me to do it in this PR?

OK, we can save it for a separate PR 👍

taoeffect

Approved!

Silver-IT added 3 commits June 5, 2024 17:33

chore: removed PROPOSAL_VARIANTS enum

dfe9844

feat: made notification and removed to create message when proposal i…

0f5d9c9

…s expiring

chore: removed useless state variable

13f132d

Silver-IT requested a review from taoeffect June 6, 2024 01:24

Silver-IT self-assigned this Jun 6, 2024

Silver-IT linked an issue Jun 6, 2024 that may be closed by this pull request

Manage proposal status consistently #1984

Closed

Silver-IT and others added 2 commits June 10, 2024 19:53

Save notification body in Object (#2052)

8972c6a

* PR for release v0.4.2 * feat: save notification body as object format --------- Co-authored-by: Greg Slepak <[email protected]>

Merge branch 'master' into 1984-manage-proposal-status-consistently

3e83930

Silver-IT changed the title ~~Manage proposal status consistently~~ Improve to manage notifications and proposals Jun 10, 2024

Silver-IT commented Jun 10, 2024

View reviewed changes

taoeffect reviewed Jun 11, 2024

View reviewed changes

historical/views/containers/chatroom/MessageInteractive.vue Outdated Show resolved Hide resolved

Silver-IT added 2 commits June 14, 2024 08:55

chore: uncommented the codes from historical MessageInteractive.vue

8b7de9c

Merge branch 'master' into 1984-manage-proposal-status-consistently

9b77f81

taoeffect requested changes Jun 18, 2024

View reviewed changes

fix: resolved conflicts

8b8b101

Silver-IT marked this pull request as draft June 21, 2024 21:05

Silver-IT added 5 commits June 25, 2024 07:21

chore: reverted changes according to the feedbackc

fea1963

fix: tiny errors

be2ceb8

fix: 2 small errors

67405c1

fix: resvoled conflicts

8e7d0d4

fix: error in using param in notifyExpiringProposal function

2e710b3

Silver-IT commented Jun 25, 2024

View reviewed changes

Silver-IT marked this pull request as ready for review June 26, 2024 20:55

Silver-IT requested a review from taoeffect June 26, 2024 21:05

taoeffect mentioned this pull request Jun 27, 2024

Ability to get contractID of person who sent a GIMessage (innerSigningContractID() function) #2125

Closed

taoeffect requested changes Jun 27, 2024

View reviewed changes

Silver-IT added 2 commits June 28, 2024 06:26

Merge branch 'master' into 1984-manage-proposal-status-consistently

4c138d3

chore: tiny update according to the feedback

3be4f34

chore: tiny update and travis retry

07f200c

taoeffect requested changes Jun 28, 2024

View reviewed changes

Silver-IT mentioned this pull request Jun 28, 2024

Notify all types of proposals in general chatroom #2138

Closed

taoeffect approved these changes Jun 28, 2024

View reviewed changes

taoeffect merged commit 41dc1f1 into master Jun 28, 2024
4 checks passed

taoeffect deleted the 1984-manage-proposal-status-consistently branch June 28, 2024 23:29

corrideat mentioned this pull request Aug 18, 2024

Fix content spoofing in notifications #2307

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve to manage notifications and proposals #2046

Improve to manage notifications and proposals #2046

Silver-IT commented Jun 6, 2024 •

edited

Loading

cypress bot commented Jun 6, 2024 •

edited

Loading

Silver-IT left a comment

Silver-IT Jun 10, 2024

Silver-IT Jun 10, 2024

Silver-IT Jun 10, 2024

Silver-IT Jun 10, 2024

taoeffect Jun 18, 2024

Silver-IT Jun 10, 2024

Silver-IT Jun 10, 2024

taoeffect left a comment

taoeffect Jun 18, 2024

taoeffect Jun 18, 2024

taoeffect Jun 18, 2024

taoeffect Jun 18, 2024

taoeffect Jun 18, 2024

taoeffect Jun 18, 2024

taoeffect Jun 18, 2024

Silver-IT left a comment

taoeffect left a comment

taoeffect Jun 27, 2024

taoeffect Jun 27, 2024

Silver-IT commented Jun 27, 2024

taoeffect left a comment

taoeffect Jun 28, 2024

Silver-IT Jun 28, 2024

taoeffect Jun 28, 2024

taoeffect left a comment

		...template,
		avatarUserID: template.avatarUserID \|\| sbp('state/vuex/getters').ourIdentityContractId,

Improve to manage notifications and proposals #2046

Improve to manage notifications and proposals #2046

Conversation

Silver-IT commented Jun 6, 2024 • edited Loading

Summary of Changes

cypress bot commented Jun 6, 2024 • edited Loading

Passing run #2489 ↗︎

Review all test suite changes for PR #2046 ↗︎

Silver-IT 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

taoeffect 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

Silver-IT left a comment

Choose a reason for hiding this comment

taoeffect left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Silver-IT commented Jun 27, 2024

taoeffect 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

taoeffect left a comment

Choose a reason for hiding this comment

Silver-IT commented Jun 6, 2024 •

edited

Loading

cypress bot commented Jun 6, 2024 •

edited

Loading