Skip to content

Commit

Permalink
[cs] #651 - Improves the notification messages shown when the occupan…
Browse files Browse the repository at this point in the history 
…t affiliation and role has been changed
  • Loading branch information
@sacurio
sacurio committed Feb 18, 2021
1 parent 9579c84 commit 09d2b4b
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 35 deletions.
38 changes: 19 additions & 19 deletions gui/muc_room_conversation_display_affiliations.go
View file
Expand Up @@ -36,7 +36,7 @@ func displayAffiliationUpdateMessage(d affiliationUpdateDisplayer, beforeReasonM
}

if reason := d.updateReason(); reason != "" {
message = i18n.Localf("%s because: %s", message, reason)
message = i18n.Localf("%s The reason given was: %s.", message, reason)
}

return message
Expand Down Expand Up @@ -86,10 +86,10 @@ func (d *affiliationUpdateDisplayData) updateReason() string {

func (d *affiliationUpdateDisplayData) displayForAffiliationRemoved() string {
if d.actor == "" {
return i18n.Localf("The %s position of %s was removed",
return i18n.Localf("The %s position of %s was removed.",

This comment has been minimized.

Sign in to view

Copy link
@olabiniV2

olabiniV2 Feb 19, 2021

Contributor

Didn't we say that in our user testing that positions can't be removed, it doesn't make sense!

This comment has been minimized.

Sign in to view

Copy link
@azlancpool

azlancpool Mar 9, 2021

Contributor

It was removed in df37e97.
It currently looks like:

coyim/gui/muc_notification_messages.go

Lines 16 to 27 in d68679f

func getAffiliationRemovedSuccessMessage(nickname string, previousAffiliation data.Affiliation) string {
switch {
case previousAffiliation.IsOwner():
return i18n.Localf("%[1]s is not an owner anymore.", nickname)
case previousAffiliation.IsAdmin():
return i18n.Localf("%[1]s is not an administrator anymore.", nickname)
case previousAffiliation.IsMember():
return i18n.Localf("%[1]s is not a member anymore.", nickname)
default:
return i18n.Localf("%[1]s is not banned anymore.", nickname)
}
}

displayNameForAffiliation(d.previousAffiliation), d.nickname)
}
return i18n.Localf("%s removed the %s position from %s",
return i18n.Localf("%s removed the %s position from %s.",
displayActorWithAffiliation(d.actor, d.actorAffiliation),
displayNameForAffiliation(d.previousAffiliation),
d.nickname,
Expand All @@ -98,20 +98,20 @@ func (d *affiliationUpdateDisplayData) displayForAffiliationRemoved() string {

func (d *affiliationUpdateDisplayData) displayForAffiliationOutcast() string {
if d.actor == "" {
return i18n.Localf("%s was banned from the room", d.nickname)
return i18n.Localf("%s was banned from the room.", d.nickname)
}
return i18n.Localf("%s banned %s from the room",
return i18n.Localf("%s banned %s from the room.",
displayActorWithAffiliation(d.actor, d.actorAffiliation),
d.nickname,
)
}

func (d *affiliationUpdateDisplayData) displayForAffiliationAdded() string {
if d.actor == "" {
return i18n.Localf("%s is now %s", d.nickname,
return i18n.Localf("%s is now %s.", d.nickname,
displayNameForAffiliationWithPreposition(d.newAffiliation))
}
return i18n.Localf("%s changed the position of %s to %s",
return i18n.Localf("%s changed the position of %s to %s.",
displayActorWithAffiliation(d.actor, d.actorAffiliation),
d.nickname,
displayNameForAffiliation(d.newAffiliation),
Expand All @@ -120,11 +120,11 @@ func (d *affiliationUpdateDisplayData) displayForAffiliationAdded() string {

func (d *affiliationUpdateDisplayData) displayForAffiliationChanged() string {
if d.actor == "" {
return i18n.Localf("The position of %s was changed from %s to %s", d.nickname,
return i18n.Localf("The position of %s was changed from %s to %s.", d.nickname,
displayNameForAffiliation(d.previousAffiliation),
displayNameForAffiliation(d.newAffiliation))
}
return i18n.Localf("%s changed the position of %s from %s to %s",
return i18n.Localf("%s changed the position of %s from %s to %s.",
displayActorWithAffiliation(d.actor, d.actorAffiliation),
d.nickname,
displayNameForAffiliation(d.previousAffiliation),
Expand All @@ -149,19 +149,19 @@ func newSelfAffiliationUpdateDisplayData(affiliationUpdate data.AffiliationUpdat

func (d *selfAffiliationUpdateDisplayData) displayForAffiliationRemoved() string {
if d.actor == "" {
return i18n.Localf("Your position of %s was removed", displayNameForAffiliation(d.previousAffiliation))
return i18n.Localf("Your position of %s was removed.", displayNameForAffiliation(d.previousAffiliation))
}
return i18n.Localf("%s removed your position as %s",
return i18n.Localf("%s removed your position as %s.",
displayActorWithAffiliation(d.actor, d.actorAffiliation),
displayNameForAffiliation(d.previousAffiliation),
)
}

func (d *selfAffiliationUpdateDisplayData) displayForAffiliationOutcast() string {
if d.actor == "" {
return i18n.Local("You were banned from the room")
return i18n.Local("You were banned from the room.")
}
return i18n.Localf("%s banned you from the room",
return i18n.Localf("%s banned you from the room.",
displayActorWithAffiliation(d.actor, d.actorAffiliation),
displayNameForAffiliation(d.actorAffiliation),
d.actor,
Expand All @@ -170,21 +170,21 @@ func (d *selfAffiliationUpdateDisplayData) displayForAffiliationOutcast() string

func (d *selfAffiliationUpdateDisplayData) displayForAffiliationAdded() string {
if d.actor == "" {
return i18n.Localf("You are now %s", displayNameForAffiliationWithPreposition(d.newAffiliation))
return i18n.Localf("You are now %s.", displayNameForAffiliationWithPreposition(d.newAffiliation))
}
return i18n.Localf("%s changed your position to %s",
return i18n.Localf("%s changed your position to %s.",
displayActorWithAffiliation(d.actor, d.actorAffiliation),
displayNameForAffiliation(d.newAffiliation),
)
}

func (d *selfAffiliationUpdateDisplayData) displayForAffiliationChanged() string {
if d.actor == "" {
return i18n.Localf("Your position was changed from %s to %s",
return i18n.Localf("Your position was changed from %s to %s.",
displayNameForAffiliation(d.previousAffiliation),
displayNameForAffiliation(d.newAffiliation))
}
return i18n.Localf("%s changed your position from %s to %s",
return i18n.Localf("%s changed your position from %s to %s.",
displayActorWithAffiliation(d.actor, d.actorAffiliation),
displayNameForAffiliation(d.previousAffiliation),
displayNameForAffiliation(d.newAffiliation))
Expand Down Expand Up @@ -233,8 +233,8 @@ func displayNameForAffiliationWithPreposition(affiliation data.Affiliation) stri
func succesAffiliationUpdateMessage(nickname string, previousAffiliation, affiliation data.Affiliation) string {
switch {
case affiliation.IsNone():
return i18n.Localf("%s is not %s anymore", nickname, displayNameForAffiliationWithPreposition(previousAffiliation))
return i18n.Localf("%s is not %s anymore.", nickname, displayNameForAffiliationWithPreposition(previousAffiliation))
default:
return i18n.Localf("The position of %s was updated to %s", nickname, displayNameForAffiliation(affiliation))
return i18n.Localf("The position of %s was updated to %s.", nickname, displayNameForAffiliation(affiliation))
}
}
4 changes: 2 additions & 2 deletions gui/muc_room_conversation_display_affiliations_roles.go
View file
Expand Up @@ -14,7 +14,7 @@ func getDisplayForOccupantAffiliationRoleUpdate(affiliationRoleUpdate data.Affil
Actor: affiliationRoleUpdate.Actor,
})

message := displayAffiliationUpdateMessage(d, i18n.Localf("and as a result the role changed from %s to %s",
message := displayAffiliationUpdateMessage(d, i18n.Localf("As a result, the role changed from %s to %s.",
displayNameForRole(affiliationRoleUpdate.PreviousRole),
displayNameForRole(affiliationRoleUpdate.NewRole)))

Expand All @@ -30,7 +30,7 @@ func getDisplayForSelfOccupantAffiliationRoleUpdate(affiliationRoleUpdate data.A
Actor: affiliationRoleUpdate.Actor,
})

message := displayAffiliationUpdateMessage(d, i18n.Localf("and as a result the role changed from %s to %s",
message := displayAffiliationUpdateMessage(d, i18n.Localf("As a result, your role changed from %s to %s.",
displayNameForRole(affiliationRoleUpdate.PreviousRole),
displayNameForRole(affiliationRoleUpdate.NewRole)))

Expand Down
28 changes: 14 additions & 14 deletions gui/muc_room_conversation_display_affiliations_test.go
View file
Expand Up @@ -28,7 +28,7 @@ func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisp
})

c.Assert(displayAffiliationUpdateMessage(d, ""), Equals,
"alex changed the position of nick to member")
"alex changed the position of nick to member.")

c.Assert(getDisplayForOccupantAffiliationUpdate(data.AffiliationUpdate{
Nickname: "robin",
Expand All @@ -39,7 +39,7 @@ func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisp
Nickname: "batman",
Affiliation: member,
},
}), Equals, "The member batman removed the member position from robin because: I'm batman")
}), Equals, "The member batman removed the member position from robin. The reason given was: I'm batman.")

c.Assert(getDisplayForOccupantAffiliationUpdate(data.AffiliationUpdate{
Nickname: "bob",
Expand All @@ -50,7 +50,7 @@ func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisp
Nickname: "alice",
Affiliation: member,
},
}), Equals, "The member alice banned bob from the room because: he was rude")
}), Equals, "The member alice banned bob from the room. The reason given was: he was rude.")

c.Assert(getDisplayForOccupantAffiliationUpdate(data.AffiliationUpdate{
Nickname: "nick",
Expand All @@ -60,7 +60,7 @@ func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisp
Nickname: "jonathan",
Affiliation: outcast,
},
}), Equals, "The outcast jonathan removed the outcast position from nick")
}), Equals, "The outcast jonathan removed the outcast position from nick.")
}

func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisplay_displayForAffiliationRemoved(c *C) {
Expand Down Expand Up @@ -91,14 +91,14 @@ func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisp
d.previousAffiliation = member
d.actor = ""
c.Assert(d.displayForAffiliationRemoved(), Equals,
"The member position of nick was removed")
"The member position of nick was removed.")

d.nickname = "007"
d.previousAffiliation = owner
d.actor = "maria"
d.actorAffiliation = owner
c.Assert(d.displayForAffiliationRemoved(), Equals,
"The owner maria removed the owner position from 007")
"The owner maria removed the owner position from 007.")
}

func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisplay_displayForAffiliationOutcast(c *C) {
Expand All @@ -110,12 +110,12 @@ func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisp
})

c.Assert(d.displayForAffiliationOutcast(), Equals,
"nick was banned from the room")
"nick was banned from the room.")

d.nickname = "jonathan"
d.actor = "maria"
c.Assert(d.displayForAffiliationOutcast(), Equals,
"maria banned jonathan from the room")
"maria banned jonathan from the room.")
}

func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisplay_displayForAffiliationAdded(c *C) {
Expand All @@ -133,21 +133,21 @@ func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisp
})

c.Assert(d.displayForAffiliationAdded(), Equals,
"nick is now a member")
"nick is now a member.")

d.nickname = "maria"
d.newAffiliation = admin
d.actor = "alberto"
d.actorAffiliation = admin
c.Assert(d.displayForAffiliationAdded(), Equals,
"The administrator alberto changed the position of maria to administrator")
"The administrator alberto changed the position of maria to administrator.")

d.nickname = "alice"
d.newAffiliation = owner
d.actor = "bob"
d.actorAffiliation = owner
c.Assert(d.displayForAffiliationAdded(), Equals,
"The owner bob changed the position of alice to owner")
"The owner bob changed the position of alice to owner.")
}

func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisplay_displayForAffiliationChanged(c *C) {
Expand All @@ -164,23 +164,23 @@ func (*MUCRoomConversationDisplayAffiliationsSuite) Test_mucRoomConversationDisp
})

c.Assert(d.displayForAffiliationChanged(), Equals,
"The position of nick was changed from administrator to member")
"The position of nick was changed from administrator to member.")

d.nickname = "maria"
d.newAffiliation = admin
d.previousAffiliation = member
d.actor = "juan"
d.actorAffiliation = member
c.Assert(d.displayForAffiliationChanged(), Equals,
"The member juan changed the position of maria from member to administrator")
"The member juan changed the position of maria from member to administrator.")

d.nickname = "alice"
d.newAffiliation = owner
d.previousAffiliation = member
d.actor = "bob"
d.actorAffiliation = member
c.Assert(d.displayForAffiliationChanged(), Equals,
"The member bob changed the position of alice from member to owner")
"The member bob changed the position of alice from member to owner.")
}

func newAffiliationFromString(s string) data.Affiliation {
Expand Down

2 comments on commit 09d2b4b

@azlancpool
Copy link
Contributor

Choose a reason for hiding this comment

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

This is part of #644

@azlancpool
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments fixed

Please sign in to comment.