#2441 - proposal notifications should have 'you' when I am the creator #2444
Conversation
group-income
|
||||||||||||||||||||||||||||
| Project |
group-income
|
| Branch Review |
sebin/task/#2441-notification-should-have-you
|
| Run status |
|
| Run duration | 11m 03s |
| Commit |
|
| Committer | Sebin Song |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
10
|
Skipped
|
0
|
Passing
|
111
|
| View all changes introduced in this branch ↗︎ | |
frontend/model/contracts/group.js
Outdated
| @@ -702,6 +702,7 @@ sbp('chelonia/defineContract', { | |||
| createdDate: meta.createdDate, | |||
| groupID: contractID, | |||
| creatorID: innerSigningContractID, | |||
| isCreator: innerSigningContractID === loggedIn.identityContractID, | |||
There was a problem hiding this comment.
Instead of isCreator, I think it'd be better to leave this as is, and add the creatorID to the PROPOSAL_CLOSED notification. Then, we can check for creator when the notification text is created. (The reason for doing it this way is to keep the contract code as less reliant on external state as possible).
frontend/model/contracts/group.js
Outdated
| sbp('gi.notifications/emit', 'PROPOSAL_CLOSED', { | ||
| createdDate: meta.createdDate, | ||
| groupID: contractID, | ||
| isCreator: loggedIn.identityContractID === proposal.creatorID, |
There was a problem hiding this comment.
|
@taoeffect Updated the PR for Ricardo's feedbacks. |
| const args = isCreator | ||
| ? null | ||
| : { | ||
| name: `${CHATROOM_MEMBER_MENTION_SPECIAL_CHAR}${data.creatorID}`, | ||
| ...LTags('strong') | ||
| } |
There was a problem hiding this comment.
It's not like args is used if isCreator is true, so this ?: isn't needed here, these changes can be reverted.
| REMOVE_MEMBER: L('{strong_}{name}{_strong} proposed to remove a member from the group. Vote now!', args), | ||
| GENERIC: L('{strong_}{name}{_strong} created a proposal. Vote now!', args) | ||
| ADD_MEMBER: isCreator | ||
| ? L('you proposed to add a member to the group.') |
There was a problem hiding this comment.
Every sentence that starts with "you" needs to be replaced with "You" because sentences should start with an uppercase letter.
| ? L('your proposal to add {member} to the group was {strong_}{closedWith}{_strong}.', args) | ||
| : L("{strong_}{name}'s{_strong} proposal to add {member} to the group was {strong_}{closedWith}{_strong}.", args), | ||
| [PROPOSAL_REMOVE_MEMBER]: isCreator | ||
| ? L('your proposal to remove {member} from the group was {strong_}{closedWith}{_strong}.', args) |
There was a problem hiding this comment.
Every sentence that starts with "your" needs to be replaced with "Your" because sentences should start with an uppercase letter.
|
@taoeffect updated the PR again for your feedbacks. |
closes #2441
Made fixes for
NEW_PROPOSALandPROPOSAL_CLOSEDnotifications.PROPOSAL_EXPIRINGdoes not need this fix because it is emitted only to those who have not voted yet. (Creator always votes for 'yes') But please let me know if I have missed anything here.