STORM-1795 Ad-hoc messages are received even when "Only friends and groups can call or IM me"


Submitter:	Jonathan Yap	Reviewers
Branch:	3.2.7	Groups:	viewer
Bugs:	STORM-1795	People:
Change Number:	None	Repository:	viewer-development

Description:

With avatar A: check on Preferences->Privacy->Only friends and groups can call or IM me

Get together with:
Avatar B (not a friend of A)
Avatar C (a friend of A)

Using avatar B establish an ad-hoc chat with A and C and write a message

Observed result: Avatar A and C receive the message
Expected result: Only avatar C receives the message

Testing Done:

See test plan in jira

Expand All

indra/newview/llimview.cpp (Diff revision 1)

What's wrong with '  != NULL ' here?

I'm a fan of conditional expressions, but this is overdoing it.

really, I think that all this could be folded into the if expression...

indra/newview/llimview.cpp (Diff revision 1)

I think there may be something else wrong with this logic: you do not have calls/IMs restricted and someone is your friend, this would not show the message (false AND true produce false), or am I missing something here?

Diff
- added Diff r2

Description:

Fixed logic error checking to see if message should be displayed or not.

Diff
- added Diff r3

Description:

Fix misspelled variable name.

Testing Done

Changed from:

Created a 3-way ad-hoc session per Description.  Avatar C does not receive the message, but does get an ad-hoc session icon, even when B initiates the ad-hoc session.  I think this is correct behavior because C may change their preferences to allow messages from everyone.  You do not want to reject the session (that was handled by a different jira recently for blocked residents), only suppress messages.

Changed to:

See test plan in jira

Description:

Pointed test plan field to jira; no need to have two unsynchronized copies

indra/newview/llimview.cpp (Diff revision 3)

Couldn't this all be folded into the 'if' without the flag variable as:

if (  !LLMuteList::getInstance()->isMuted(other_participant_id, LLMute::flagTextChat)
    && ! (gSavedSettings.getBOOL("VoiceCallsFriendsOnly") && (LLAvatarTracker::instance().getBuddyInfo(other_participant_id) == NULL))

(that probably won't format well)

Jonathan Yap Jan. 23, 2012, 9:13 a.m. (Jan. 23, 2012, 9:13 a.m.)

There are two issues:
1) Clarity of code for future programmers (I think having the flag variable helps here)

2) Does your folded-in solution work properly for all 4 cases of VoiceCalls true/false and friend/not-friend?  Am I having a dumb moment or does the case of VoiceCalls=false and is-friend=false not work properly here (!(false && false)) = false, when it should be true?

Diff
- added Diff r4

Description:

Slight reformatting/logic change.  I think this is a good compromise between my too-wordy multi-line if statement and Oz's compact everything into one if statement approach.  This is readable and doesn't waste space.

Ship It!

status changed from pending to submitted

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

This change has been marked as submitted.