Addressed fourth round of review comments

pontoon/base/models/comment.py

@@ -30,7 +30,7 @@ def serialize(self):
         return {
             "author": self.author.name_or_email,
             "username": self.author.username,
-            "user_status": self.author.locale_status(self.locale),
+            "user_status": self.author.status(self.locale),
             "user_gravatar_url_small": self.author.gravatar_url(88),
             "created_at": self.timestamp.strftime("%b %d, %Y %H:%M"),
             "date_iso": self.timestamp.isoformat(),

pontoon/base/models/user.py

@@ -194,7 +194,7 @@ def user_locale_role(self, locale):
         return "Contributor"
 
 
-def user_locale_status(self, locale):
+def user_status(self, locale):
     if self.username == "Imported":
         return ("", "")
     if self.is_superuser:
@@ -439,7 +439,7 @@ def latest_action(self):
 User.add_to_class("translated_projects", user_translated_projects)
 User.add_to_class("role", user_role)
 User.add_to_class("locale_role", user_locale_role)
-User.add_to_class("locale_status", user_locale_status)
+User.add_to_class("status", user_status)
 User.add_to_class("contributed_translations", contributed_translations)
 User.add_to_class("top_contributed_locale", top_contributed_locale)
 User.add_to_class("can_translate", can_translate)

pontoon/base/tests/models/test_user.py

@@ -64,22 +64,22 @@ def test_user_locale_role(user_a, user_b, user_c, locale_a):
 
 
 @pytest.mark.django_db
-def test_user_locale_status(user_a, user_b, user_c, locale_a):
+def test_user_status(user_a, user_b, user_c, locale_a):
     # New User
-    assert user_a.locale_status(locale_a)[1] == "New User"
+    assert user_a.status(locale_a)[1] == "New User"
 
     # Fake user object
     imported = User(username="Imported")
-    assert imported.locale_status(locale_a)[1] == ""
+    assert imported.status(locale_a)[1] == ""
 
     # Admin
     user_a.is_superuser = True
-    assert user_a.locale_status(locale_a)[1] == "Admin"
+    assert user_a.status(locale_a)[1] == "Admin"
 
     # Manager
     locale_a.managers_group.user_set.add(user_b)
-    assert user_b.locale_status(locale_a)[1] == "Manager"
+    assert user_b.status(locale_a)[1] == "Manager"
 
     # Translator
     locale_a.translators_group.user_set.add(user_c)
-    assert user_c.locale_status(locale_a)[1] == "Translator"
+    assert user_c.status(locale_a)[1] == "Translator"

pontoon/base/views.py

@@ -420,7 +420,7 @@ def get_translation_history(request):
                 "uid": u.id,
                 "username": u.username,
                 "user_gravatar_url_small": u.gravatar_url(88),
-                "user_status": u.locale_status(locale),
+                "user_status": u.status(locale),
                 "date": t.date,
                 "approved_user": User.display_name_or_blank(t.approved_user),
                 "approved_date": t.approved_date,

translate/src/api/comment.ts

@@ -9,7 +9,7 @@ import { keysToCamelCase } from './utils/keysToCamelCase';
 export type TranslationComment = {
   readonly author: string;
   readonly username: string;
-  readonly userStatus: string;
+  readonly userStatus: string[];
   readonly userGravatarUrlSmall: string;
   readonly createdAt: string;
   readonly dateIso: string;

translate/src/api/translation.ts

@@ -44,7 +44,7 @@ export type HistoryTranslation = {
   readonly user: string;
   readonly username: string;
   readonly userGravatarUrlSmall: string;
-  readonly userStatus: string;
+  readonly userStatus: string[];
   readonly comments: Array<TranslationComment>;
 };
 

translate/src/api/user.ts

@@ -30,6 +30,7 @@ export type ApiUserData = {
   name_or_email?: string;
   email?: string;
   username?: string;
+  date_joined?: string;
   manager_for_locales?: string[];
   translator_for_locales?: string[];
   translator_for_projects?: Record<string, boolean>;

translate/src/hooks/useUserStatus.test.js

@@ -0,0 +1,88 @@
+import React from 'react';
+import sinon from 'sinon';
+
+import { USER } from '~/modules/user';
+import * as Hooks from '~/hooks';
+
+import { useUserStatus } from './useUserStatus';
+
+beforeAll(() => {
+  sinon.stub(Hooks, 'useAppSelector');
+  sinon.stub(React, 'useContext').returns({ code: 'mylocale' });
+});
+afterAll(() => {
+  Hooks.useAppSelector.restore();
+  React.useContext.restore();
+});
+
+const fakeSelector = (user) => (sel) =>
+  sel({
+    [USER]: user,
+  });
+
+describe('useUserStatus', () => {
+  it('returns empty parameters for non-authenticated users', () => {
+    Hooks.useAppSelector.callsFake(fakeSelector({ isAuthenticated: false })),
+      expect(useUserStatus()).toStrictEqual(['', '']);
+  });
+
+  it('returns [ADMIN, Admin] if user has admin permissions', () => {
+    Hooks.useAppSelector.callsFake(
+      fakeSelector({
+        isAuthenticated: true,
+        isAdmin: true,
+        managerForLocales: ['mylocale'],
+        translatorForLocales: [],
+      }),
+    );
+    expect(useUserStatus()).toStrictEqual(['ADMIN', 'Admin']);
+  });
+
+  it('returns [MNGR, Manager] if user is a manager of the locale', () => {
+    Hooks.useAppSelector.callsFake(
+      fakeSelector({
+        isAuthenticated: true,
+        managerForLocales: ['mylocale'],
+        translatorForLocales: [],
+      }),
+    );
+    expect(useUserStatus()).toStrictEqual(['MNGR', 'Manager']);
+  });
+
+  it('returns [TRNSL, Translator] if user is a translator for the locale', () => {
+    Hooks.useAppSelector.callsFake(
+      fakeSelector({
+        isAuthenticated: true,
+        managerForLocales: [],
+        translatorForLocales: ['mylocale'],
+      }),
+    );
+    expect(useUserStatus()).toStrictEqual(['TRNSL', 'Translator']);
+  });
+
+  it('returns [NEW, New User] if user created their account within the last 3 months', () => {
+    const dateJoined = new Date();
+    dateJoined.setMonth(dateJoined.getMonth() - 2);
+    Hooks.useAppSelector.callsFake(
+      fakeSelector({
+        isAuthenticated: true,
+        managerForLocales: [],
+        translatorForLocales: [],
+        dateJoined: dateJoined,
+      }),
+    );
+    expect(useUserStatus()).toStrictEqual(['NEW', 'New User']);
+
+    // Set join date to be 6 months ago (no longer a new user)
+    dateJoined.setMonth(dateJoined.getMonth() - 6);
+    Hooks.useAppSelector.callsFake(
+      fakeSelector({
+        isAuthenticated: true,
+        managerForLocales: [],
+        translatorForLocales: [],
+        dateJoined: dateJoined,
+      }),
+    );
+    expect(useUserStatus()).toStrictEqual(['', '']);
+  });
+});

translate/src/hooks/useLocaleRole.ts → translate/src/hooks/useUserStatus.ts

@@ -7,7 +7,7 @@ import { useAppSelector } from '~/hooks';
 /**
  * Return the user's status within the given locale, to display on the user banner
  */
-export function useLocaleRole(): Array<string> {
+export function useUserStatus(): Array<string> {
   const { code } = useContext(Locale);
   const {
     isAuthenticated,
@@ -25,19 +25,19 @@ export function useLocaleRole(): Array<string> {
     return ['ADMIN', 'Admin'];
   }
 
-  if (Object.hasOwnProperty.call(managerForLocales, code)) {
-    return ['MNGR', 'Manger'];
+  if (managerForLocales.includes(code)) {
+    return ['MNGR', 'Manager'];
   }
 
-  if (Object.hasOwnProperty.call(translatorForLocales, code)) {
+  if (translatorForLocales.includes(code)) {
     return ['TRNSL', 'Translator'];
   }
 
   const dateJoinedObj = new Date(dateJoined);
-  let newUserDate = dateJoinedObj;
-  newUserDate.setMonth(dateJoinedObj.getMonth() - 3);
-  if (dateJoinedObj < newUserDate) {
-    return ['NEW USER', 'New User'];
+  let threeMonthsAgo = new Date();
+  threeMonthsAgo.setMonth(threeMonthsAgo.getMonth() - 3);
+  if (dateJoinedObj > threeMonthsAgo) {
+    return ['NEW', 'New User'];
   }
 
   return ['', ''];

translate/src/modules/comments/components/AddComment.tsx

@@ -30,7 +30,7 @@ import type { MentionUser } from '~/api/user';
 import { MentionUsers } from '~/context/MentionUsers';
 import type { UserState } from '~/modules/user';
 import { UserAvatar } from '~/modules/user';
-import { useLocaleRole } from '~/hooks/useLocaleRole';
+import { useUserStatus } from '~/hooks/useUserStatus';
 
 import './AddComment.css';
 import { MentionList } from './MentionList';
@@ -87,7 +87,7 @@ export function AddComment({
   const [mentionIndex, setMentionIndex] = useState(0);
   const [mentionSearch, setMentionSearch] = useState('');
   const [requireUsers, setRequireUsers] = useState(false);
-  const role = useLocaleRole();
+  const role = useUserStatus();
 
   const { initMentions, mentionUsers } = useContext(MentionUsers);
   const [slateKey, resetValue] = useReducer((key) => key + 1, 0);

translate/src/modules/comments/components/Comment.tsx

@@ -9,7 +9,6 @@ import type { TranslationComment } from '~/api/comment';
 import { UserAvatar } from '~/modules/user';
 
 import './Comment.css';
-import '../../user/components/UserAvatar.css';
 
 type Props = {
   comment: TranslationComment;

translate/src/modules/comments/components/CommentsList.tsx

@@ -7,7 +7,6 @@ import { useAddCommentAndRefresh } from '../hooks';
 import { AddComment } from './AddComment';
 import { Comment } from './Comment';
 import './CommentsList.css';
-import { useLocaleRole } from '~/hooks/useLocaleRole';
 
 type Props = {
   translation: HistoryTranslation;
@@ -19,7 +18,6 @@ export function CommentsList({
   user,
 }: Props): React.ReactElement<'div'> {
   const onAddComment = useAddCommentAndRefresh(translation);
-  const role = useLocaleRole();
 
   return (
     <div className='comments-list'>

translate/src/modules/history/components/HistoryTranslation.tsx

@@ -14,7 +14,6 @@ import { withActionsDisabled } from '~/utils';
 import { useTranslator } from '~/hooks/useTranslator';
 
 import './HistoryTranslation.css';
-import '../../user/components/UserAvatar.css';
 
 type Props = {
   entity: Entity;

translate/src/modules/user/components/UserAvatar.css

@@ -4,6 +4,15 @@
   top: 3px;
 }
 
+.history .translation .user-avatar img {
+  border-radius: 6px;
+  border: 2px solid var(--icon-border-1);
+}
+
+.history .translation:hover .user-avatar img {
+  border-color: var(--translation-border);
+}
+
 .user-avatar .avatar-container {
   position: relative;
   display: inline-block;
@@ -44,16 +53,3 @@
 .user-avatar .avatar-container .user-status-banner.new-user {
   color: var(--status-fuzzy);
 }
-
-.history .translation:hover .user-status-banner {
-  border-color: var(--translation-border);
-}
-
-.history .translation .user-avatar img {
-  border-radius: 6px;
-  border: 2px solid var(--icon-border-1);
-}
-
-.history .translation:hover .user-avatar img {
-  border-color: var(--translation-border);
-}

translate/src/modules/user/components/UserAvatar.tsx

@@ -1,11 +1,13 @@
 import React from 'react';
 import { Localized } from '@fluent/react';
 
+import './UserAvatar.css';
+
 type Props = {
   username: string;
   title?: string;
   imageUrl: string;
-  userStatus: string | string[];
+  userStatus: string[];
 };
 
 export function UserAvatar(props: Props): React.ReactElement<'div'> {
@@ -25,7 +27,7 @@ export function UserAvatar(props: Props): React.ReactElement<'div'> {
           <Localized id='user-UserAvatar--alt-text' attrs={{ alt: true }}>
             <img src={imageUrl} alt='User Profile' height='44' width='44' />
           </Localized>
-          {userStatus && (
+          {role && (
             <span
               className={`user-status-banner ${tooltip.toLowerCase().split(' ').join('-')}`}
               title={tooltip}