Institution addUserAsMember lang logic is flawed

Bug #1703608 reported by Nelson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Medium
Nelson

Bug Description

The logic in charge of selecting the lang for the message does not work as expected.
The condition:

if ($lang = get_account_preference($user->id, 'lang')) {
            // The user has a preset lang preference so we will use this
        }
        else if ($this->lang != 'default') {
            // The user hasn't been added yet, so we have to manually use this institution's lang
            $lang = $this->lang;
        }
        else {
            $lang = get_user_language($user->id);
        }

gets always sent in english if the site default lang is french.

To reproduce it:
You set another language than english as default language. You an institution with default lang and then you add a user (his lang preference is not setting) into this institution. He is going to receive an english message in that case and also in the case that the institution language is explicitly set to the additional lang.

That is corrected with the following:

$lang = get_account_preference($user->id, 'lang');

        if ($lang == 'default') {
            // The user has not a preset lang preference so we will use the institution if it has one.
            $institution_lang = get_record_sql(
              "SELECT value FROM {institution_config} ic where ic.institution= ? and ic.field = 'lang' ",
              [$this->name],
              IGNORE_MULTIPLE
            );
            $this->lang = $institution_lang->value? $institution_lang->value: 'default';
            if ($this->lang != 'default') {
                // The user hasn't been added yet, so we have to manually use this institution's lang
                $lang = $this->lang;
            }
            else {
                $lang = get_config('lang')? get_config('lang'): '';
            }
        }

We have also opened a PR in github: https://github.com/MaharaProject/mahara/pull/7

Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

Thank you very much for a fix for this issue, Nelson.

Cecilia, can you please pull the patch into Gerrit and ensure that Nelson remains the author?

Nelson, we don't use Github pull requests as Github is only a mirror. If you want to get future changes into the Mahara code review system directly, you can find information on how to set up the connection to Gerrit at https://wiki.mahara.org/wiki/Developer_Area/Contributing_Code

Changed in mahara:
importance: Undecided → Medium
milestone: none → 17.10.0
assignee: nobody → Cecilia Vela Gurovic (ceciliavg)
Revision history for this message
Nelson (nmoller-c) wrote :

Thanks Kristina, I'll use gerrit the next time.
You are doing a great job so the least we can do is to follow those procedures.

Have a nice day.
N.

Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :

Hello Nelson. This is much appreciated. Thank you.

Changed in mahara:
status: New → Triaged
Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/7877

Changed in mahara:
assignee: Cecilia Vela Gurovic (ceciliavg) → nobody
Changed in mahara:
status: Triaged → In Progress
assignee: nobody → Nelson (nmoller-c)
Revision history for this message
Kristina Hoeppner (kris-hoeppner) wrote :
Download full text (3.2 KiB)

Hi Nelson, Cecilia and I had a closer look at your change and as far as we can see it concerns the institution confirmation message. Cecilia is making that clearer in the commit message.

If you want to pull the patch and make the change that she suggested, you can do so by pulling the patch directly from Gerrit. If you leave the changeID in place, it will make a new patchset in the review rather than an entirely new patch.

There are a few more messages that are concerned if you want to correct them as well: Bug 1705162.

What to test:

1. Install 4 language packs besides having the default English one. Unless English is specifically mentioned as language to test with, do not use it as one of the languages and allow users to be in multiple institutions.
2. Do not change the site admin's language.
3. Select language 1 (L1) for the site.
4. Set up Institution A with language 2 (L2).
5. Set up Institution B with language 4 (L4).

Test scenarios from here on:
A)
6. Set up another user on the site in "No institution" and do not set their language. Leave the language on the default setting, i.e. that should be the site setting.
7. As site admin, add the user to Institution A.
8. Expected result: The institution confirmation notification is sent in L2.

B)
6. Set up another user on the site and do not set their language. Leave the language on the default setting, i.e. that should be the site setting.
7. Change the language of the site admin account to language 3 (L3).
8. As site admin, add the user to Institution 2.
9. Expected result: The institution confirmation notification is sent in L2.

C)
6. Set up another user on the site and change their language to L3.
7. As site admin (being on L1 or English), add the user to Institution A.
8. Expected result: The institution confirmation notification is sent in L3 as that is the user's chosen language preference.

D)
5. Set up another user in Institution B and leave the account settings on the default language.
6. As site admin (being on L1 or English), add the user to Institution A.
7. Expected result: The institution confirmation notification is sent in L2.

E)
5. Set up another user in Institution B and change the account settings to use L3, i.e. not the institution's language.
6. As site admin (being on L1 or English), add the user to Institution A.
7. Expected result: The institution confirmation notification is sent in L3.

F)
6. Set up another user in "No institution" and do not change their language preference.
6. Set up an institution admin in Institution A and change their personal language to L1.
7. As institution admin, invite the user to your institution. The invitation message is not displayed in L2 or L3 but that is something to resolve in bug 1705162.
8. As user, accept the invitation.
9. Expected result: The institution confirmation notification is sent in L2.

G)
6. Set up another user in "No institution" and change their language to L3.
6. Set up an institution admin in Institution A and change their personal language to L1.
7. As institution admin, invite the user to your institution. The invitation message is not displayed in L2 or L3 but that is something to resolve in bug 1705162...

Read more...

Revision history for this message
Nelson (nmoller-c) wrote :

Thanks to both of you. I agree with your changes. What matters is to get the right message, for the next one I'll make the setup to pull the change to gerrit myself :) .

The invitation case seems to be handled differently. I had a quick look and I did not find some point where it looks at the institution lang.

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/7877
Committed: https://git.mahara.org/mahara/mahara/commit/4078222d49fca56c359f1640a608265380f5872a
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 4078222d49fca56c359f1640a608265380f5872a
Author: Nelson Moller <email address hidden>
Date: Mon Jul 17 15:12:05 2017 +1200

Bug 1703608: Selecting the lang for inst membership confirmation messages

Choosing the correct language for the confirmation message sent when
a user is added to an institution that has a different language than
the site default.

behatnotneeded

Change-Id: I832a18777654dc82d506e3d3a1238a74c6a6bdd7

Robert Lyon (robertl-9)
Changed in mahara:
status: In Progress → Fix Committed
Robert Lyon (robertl-9)
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.