chore: address PR reviews

ts/session/utils/job_runners/jobs/AvatarDownloadJob.ts

@@ -17,7 +17,7 @@ import {
 } from '../PersistedJob';
 
 const defaultMsBetweenRetries = 10000;
-const defaultMaxAttemps = 3;
+const defaultMaxAttempts = 3;
 
 /**
  * Returns true if the provided conversationId is a private chat and that we should add an Avatar Download Job to the list of jobs to run.
@@ -74,11 +74,7 @@ class AvatarDownloadJob extends PersistedJob<AvatarDownloadPersistedData> {
     Partial<
       Pick<
         AvatarDownloadPersistedData,
-        | 'nextAttemptTimestamp'
+        'nextAttemptTimestamp' | 'identifier' | 'maxAttempts' | 'currentRetry'
-        | 'identifier'
-        | 'maxAttempts'
-        | 'delayBetweenRetries'
-        | 'currentRetry'
       >
     >) {
     super({
@@ -86,7 +82,7 @@ class AvatarDownloadJob extends PersistedJob<AvatarDownloadPersistedData> {
       identifier: identifier || v4(),
       conversationId,
       delayBetweenRetries: defaultMsBetweenRetries,
-      maxAttempts: isNumber(maxAttempts) ? maxAttempts : defaultMaxAttemps,
+      maxAttempts: isNumber(maxAttempts) ? maxAttempts : defaultMaxAttempts,
       nextAttemptTimestamp: nextAttemptTimestamp || Date.now() + defaultMsBetweenRetries,
       currentRetry: isNumber(currentRetry) ? currentRetry : 0,
     });

ts/session/utils/job_runners/jobs/GroupInviteJob.ts

@@ -24,6 +24,7 @@ import { MessageQueue } from '../../../sending';
 import { NetworkTime } from '../../../../util/NetworkTime';
 import { SubaccountUnrevokeSubRequest } from '../../../apis/snode_api/SnodeRequestTypes';
 import { GroupSync } from './GroupSyncJob';
+import { DURATION } from '../../../constants';
 
 const defaultMsBetweenRetries = 10000;
 const defaultMaxAttempts = 1;
@@ -34,7 +35,7 @@ type JobExtraArgs = {
   inviteAsAdmin: boolean;
   /**
    * When inviting a member, we usually only want to sent a message to his swarm.
-   * In the case of a invitation resend process though, we also want to make sure his token is unrevoked from the group's swarm.
+   * In the case of an invitation resend process though, we also want to make sure his token is unrevoked from the group's swarm.
    *
    */
   forceUnrevoke: boolean;
@@ -151,11 +152,7 @@ class GroupInviteJob extends PersistedJob<GroupInvitePersistedData> {
     Partial<
       Pick<
         GroupInvitePersistedData,
-        | 'nextAttemptTimestamp'
+        'nextAttemptTimestamp' | 'identifier' | 'maxAttempts' | 'currentRetry'
-        | 'identifier'
-        | 'maxAttempts'
-        | 'delayBetweenRetries'
-        | 'currentRetry'
       >
     >) {
     super({
@@ -298,7 +295,7 @@ class GroupInviteJob extends PersistedJob<GroupInvitePersistedData> {
   }
 
   public getJobTimeoutMs(): number {
-    return 15000;
+    return 15 * DURATION.SECONDS;
   }
 }
 
@@ -321,7 +318,7 @@ function updateFailedStateForMember(groupPk: GroupPubkeyType, member: PubkeyType
   if (!thisGroupFailure) {
     thisGroupFailure = {
       failedMembers: [],
-      debouncedCall: debounce(displayFailedInvitesForGroup, 1000), // TODO change to 5000
+      debouncedCall: debounce(displayFailedInvitesForGroup, 5 * DURATION.SECONDS),
     };
   }
 

ts/session/utils/job_runners/jobs/GroupPendingRemovalsJob.ts

@@ -36,6 +36,7 @@ import {
   WithSecretKey,
 } from '../../../types/with';
 import { groupInfoActions } from '../../../../state/ducks/metaGroups';
+import { DURATION } from '../../../constants';
 
 const defaultMsBetweenRetries = 10000;
 const defaultMaxAttempts = 1;
@@ -98,11 +99,7 @@ class GroupPendingRemovalsJob extends PersistedJob<GroupPendingRemovalsPersisted
     Partial<
       Pick<
         GroupPendingRemovalsPersistedData,
-        | 'nextAttemptTimestamp'
+        'nextAttemptTimestamp' | 'identifier' | 'maxAttempts' | 'currentRetry'
-        | 'identifier'
-        | 'maxAttempts'
-        | 'delayBetweenRetries'
-        | 'currentRetry'
       >
     >) {
     super({
@@ -135,7 +132,7 @@ class GroupPendingRemovalsJob extends PersistedJob<GroupPendingRemovalsPersisted
         return RunJobResult.Success;
       }
       const deleteMessagesOfMembers = pendingRemovals
-        .filter(m => m.removedStatus === 'REMOVED_MEMBER_AND_MESSAGES')
+        .filter(m => m.memberStatus === 'REMOVED_MEMBER_AND_MESSAGES')
         .map(m => m.pubkeyHex);
 
       const sessionIdsHex = pendingRemovals.map(m => m.pubkeyHex);
@@ -274,7 +271,7 @@ class GroupPendingRemovalsJob extends PersistedJob<GroupPendingRemovalsPersisted
   }
 
   public getJobTimeoutMs(): number {
-    return 15000;
+    return 15 * DURATION.SECONDS;
   }
 }
 

ts/session/utils/job_runners/jobs/GroupPromoteJob.ts

@@ -18,6 +18,7 @@ import {
   RunJobResult,
 } from '../PersistedJob';
 import { MessageQueue } from '../../../sending';
+import { DURATION } from '../../../constants';
 
 const defaultMsBetweenRetries = 10000;
 const defaultMaxAttempts = 1;
@@ -62,11 +63,7 @@ class GroupPromoteJob extends PersistedJob<GroupPromotePersistedData> {
     Partial<
       Pick<
         GroupPromotePersistedData,
-        | 'nextAttemptTimestamp'
+        'nextAttemptTimestamp' | 'identifier' | 'maxAttempts' | 'currentRetry'
-        | 'identifier'
-        | 'maxAttempts'
-        | 'delayBetweenRetries'
-        | 'currentRetry'
       >
     >) {
     super({
@@ -153,7 +150,7 @@ class GroupPromoteJob extends PersistedJob<GroupPromotePersistedData> {
   }
 
   public getJobTimeoutMs(): number {
-    return 15000;
+    return 15 * DURATION.SECONDS;
   }
 }
 

ts/session/utils/job_runners/jobs/GroupSyncJob.ts

@@ -157,9 +157,9 @@ async function pushChangesToGroupSwarmIfNeeded({
   ]);
 
   const result = await MessageSender.sendEncryptedDataToSnode({
-    // Note: this is on purpose that supplementalKeysSubRequest is before pendingConfigRequests
+    // Note: this is on purpose that supplementalKeysSubRequest is before pendingConfigRequests.
-    // as this is to avoid a race condition where a device is polling right
+    // This is to avoid a race condition where a device is polling while we
-    // while we are posting the configs (already encrypted with the new keys)
+    // are posting the configs (already encrypted with the new keys)
     sortedSubRequests,
     destination: groupPk,
     method: 'sequence',
@@ -198,7 +198,7 @@ async function pushChangesToGroupSwarmIfNeeded({
 
 class GroupSyncJob extends PersistedJob<GroupSyncPersistedData> {
   constructor({
-    identifier, // this has to be the pubkey to which we
+    identifier, // this has to be the group's pubkey
     nextAttemptTimestamp,
     maxAttempts,
     currentRetry,
@@ -241,8 +241,6 @@ class GroupSyncJob extends PersistedJob<GroupSyncPersistedData> {
         groupPk: thisJobDestination,
         extraStoreRequests: [],
       });
-
-      // eslint-disable-next-line no-useless-catch
     } catch (e) {
       window.log.warn('GroupSyncJob failed with', e.message);
       return RunJobResult.RetryJobIfPossible;
@@ -251,7 +249,7 @@ class GroupSyncJob extends PersistedJob<GroupSyncPersistedData> {
         `GroupSyncJob ${ed25519Str(thisJobDestination)} run() took ${Date.now() - start}ms`
       );
 
-      // this is a simple way to make sure whatever happens here, we update the lastest timestamp.
+      // this is a simple way to make sure whatever happens here, we update the latest timestamp.
       // (a finally statement is always executed (no matter if exception or returns in other try/catch block)
       this.updateLastTickTimestamp();
     }

ts/session/utils/job_runners/jobs/UserSyncJob.ts

@@ -188,7 +188,7 @@ class UserSyncJob extends PersistedJob<UserSyncPersistedData> {
     } finally {
       window.log.debug(`UserSyncJob run() took ${Date.now() - start}ms`);
 
-      // this is a simple way to make sure whatever happens here, we update the lastest timestamp.
+      // this is a simple way to make sure whatever happens here, we update the latest timestamp.
       // (a finally statement is always executed (no matter if exception or returns in other try/catch block)
       this.updateLastTickTimestamp();
     }

ts/state/selectors/groups.ts

@@ -108,8 +108,12 @@ function getMemberPromotionNotSent(state: StateType, pubkey: PubkeyType, convo?:
 
 function getMemberPendingRemoval(state: StateType, pubkey: PubkeyType, convo?: GroupPubkeyType) {
   const members = getMembersOfGroup(state, convo);
-  const removedStatus = findMemberInMembers(members, pubkey)?.removedStatus;
+  const removedStatus = findMemberInMembers(members, pubkey)?.memberStatus;
-  return removedStatus !== 'NOT_REMOVED';
+  return (
+    removedStatus === 'REMOVED_UNKNOWN' ||
+    removedStatus === 'REMOVED_MEMBER' ||
+    removedStatus === 'REMOVED_MEMBER_AND_MESSAGES'
+  );
 }
 
 export function getLibMembersCount(state: StateType, convo?: GroupPubkeyType): Array<string> {
@@ -317,32 +321,38 @@ export function useStateOf03GroupMembers(convoId?: string) {
     switch (item.memberStatus) {
       case 'INVITE_FAILED':
       case 'INVITE_NOT_SENT':
-        stateSortingOrder = -5;
+        stateSortingOrder = -50;
         break;
       case 'INVITE_SENDING':
-        stateSortingOrder = -4;
+        stateSortingOrder = -40;
         break;
       case 'INVITE_SENT':
-        stateSortingOrder = -3;
+        stateSortingOrder = -30;
+        break;
+      case 'REMOVED_UNKNOWN': // fallback, hopefully won't happen in production
+      case 'REMOVED_MEMBER': // we want pending removal members at the end
+      case 'REMOVED_MEMBER_AND_MESSAGES':
+        stateSortingOrder = -20;
         break;
       case 'PROMOTION_FAILED':
       case 'PROMOTION_NOT_SENT':
-        stateSortingOrder = -2;
+        stateSortingOrder = -15;
         break;
       case 'PROMOTION_SENDING':
-        stateSortingOrder = -1;
+        stateSortingOrder = -10;
         break;
       case 'PROMOTION_SENT':
         stateSortingOrder = 0;
         break;
       case 'PROMOTION_ACCEPTED':
-        stateSortingOrder = 1;
+        stateSortingOrder = 10;
         break;
       case 'INVITE_ACCEPTED':
-        stateSortingOrder = 2;
+        stateSortingOrder = 20;
         break;
-      case 'UNKNOWN':
+      case 'INVITE_UNKNOWN': // fallback, hopefully won't happen in production
-        stateSortingOrder = 5; // just a fallback, hopefully won't happen in production
+      case 'PROMOTION_UNKNOWN': // fallback, hopefully won't happen in production
+        stateSortingOrder = 50;
         break;
 
       default:

ts/test/session/unit/libsession_wrapper/libsession_wrapper_metagroup_test.ts

@@ -25,7 +25,6 @@ function emptyMember(pubkeyHex: PubkeyType): GroupMemberGet {
       url: null,
     },
     nominatedAdmin: false,
-    removedStatus: 'NOT_REMOVED',
     pubkeyHex,
   };
 }
@@ -299,8 +298,7 @@ describe('libsession_metagroup', () => {
       expect(metaGroupWrapper.memberGetAll().length).to.be.deep.eq(1);
       const expected: GroupMemberGet = {
         ...emptyMember(member),
-        removedStatus: 'REMOVED_MEMBER_AND_MESSAGES',
+        memberStatus: 'REMOVED_MEMBER_AND_MESSAGES',
-        memberStatus: 'INVITE_ACCEPTED', // marking a member as pending removal auto-marks him as accepted (so we don't retry sending an invite)
       };
       expect(metaGroupWrapper.memberGetAll().length).to.be.deep.eq(1);
       expect(metaGroupWrapper.memberGetAll()[0]).to.be.deep.eq(expected);
@@ -312,8 +310,7 @@ describe('libsession_metagroup', () => {
       expect(metaGroupWrapper.memberGetAll().length).to.be.deep.eq(1);
       const expected: GroupMemberGet = {
         ...emptyMember(member),
-        removedStatus: 'REMOVED_MEMBER',
+        memberStatus: 'REMOVED_MEMBER',
-        memberStatus: 'INVITE_ACCEPTED', // marking a member as pending removal auto-marks him as accepted (so we don't retry sending an invite)
       };
       expect(metaGroupWrapper.memberGetAll()).to.be.deep.eq([expected]);
     });