Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Room settings-Members added/deleted in a room in offline mode are not grayed out/crossed #35355

Merged
merged 24 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1909,6 +1909,12 @@ export default {
genericUpdateReportFieldFailureMessage: 'Unexpected error while updating the field, please try again later',
genericUpdateReporNameEditFailureMessage: 'Unexpected error while renaming the report, please try again later',
noActivityYet: 'No activity yet',
people: {
error: {
genericAdd: 'There was a problem adding this room member.',
genericRemove: 'There was a problem removing that room member.',
Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds a little bit of, especially that room member part. Did someone confirm this texts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if they're correct, I just copy the texts of workspace here. @deetergp Can you help confirm?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what you have here is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, let's add the Waiting for copy label and get official @Expensify/marketing sign-off.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I don't think the "Waiting for copy" label works on pull requests! It looks like this is for a generic error message and we can't drill down into more specific reasoning?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, where will these messages show up exactly and how will they look?

Not knowing much else, I'd suggest slight tweaks:

  • There was a problem adding this member to the room.
  • There was a problem removing this member from the room.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jamesdeanexpensify Thanks for your suggestion. What about Spanish?

Copy link
Contributor

Choose a reason for hiding this comment

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

Getting a check here.

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 already in the code it seems. I'm not an engineer, but an engineer confirmed it. @deetergp are you able to see the conversation here? Thanks!

},
},
},
chronos: {
oooEventSummaryFullDay: ({summary, dayCount, date}: OOOEventSummaryFullDayParams) => `${summary} for ${dayCount} ${dayCount === 1 ? 'day' : 'days'} until ${date}`,
Expand Down
6 changes: 6 additions & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1935,6 +1935,12 @@ export default {
genericUpdateReportFieldFailureMessage: 'Error inesperado al actualizar el campo. Por favor, inténtalo más tarde',
genericUpdateReporNameEditFailureMessage: 'Error inesperado al cambiar el nombre del informe. Vuelva a intentarlo más tarde.',
noActivityYet: 'Sin actividad todavía',
people: {
error: {
genericAdd: '',
genericRemove: '',
},
},
},
chronos: {
oooEventSummaryFullDay: ({summary, dayCount, date}: OOOEventSummaryFullDayParams) => `${summary} por ${dayCount} ${dayCount === 1 ? 'día' : 'días'} hasta el ${date}`,
Expand Down
79 changes: 70 additions & 9 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ import type {Route} from '@src/ROUTES';
import ROUTES from '@src/ROUTES';
import type {PersonalDetails, PersonalDetailsList, PolicyReportField, RecentlyUsedReportFields, ReportActionReactions, ReportMetadata, ReportUserIsTyping} from '@src/types/onyx';
import type {Decision, OriginalMessageIOU} from '@src/types/onyx/OriginalMessage';
import type {NotificationPreference, WriteCapability} from '@src/types/onyx/Report';
import type {NotificationPreference, PendingAccount, WriteCapability} from '@src/types/onyx/Report';
import type Report from '@src/types/onyx/Report';
import type {Message, ReportActionBase, ReportActions} from '@src/types/onyx/ReportAction';
import type ReportAction from '@src/types/onyx/ReportAction';
Expand Down Expand Up @@ -2302,27 +2302,48 @@ function inviteToRoom(reportID: string, inviteeEmailsToAccountIDs: Record<string
const logins = inviteeEmails.map((memberLogin) => OptionsListUtils.addSMSDomainIfPhoneNumber(memberLogin));
const newPersonalDetailsOnyxData = PersonalDetailsUtils.getNewPersonalDetailsOnyxData(logins, inviteeAccountIDs);

const optimisticPendingAccounts: Record<string, PendingAccount> = {};
const successPendingAccounts: Record<string, PendingAccount> = {};
const failurePendingAccounts: Record<string, PendingAccount> = {};

inviteeAccountIDs.forEach((accountID) => {
optimisticPendingAccounts[accountID] = {pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD};
successPendingAccounts[accountID] = {pendingAction: null};
failurePendingAccounts[accountID] = {
errors: ErrorUtils.getMicroSecondOnyxError('report.people.error.genericAdd'),
};
});

const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
participantAccountIDs: participantAccountIDsAfterInvitation,
visibleChatMemberAccountIDs: visibleMemberAccountIDsAfterInvitation,
pendingAccounts: optimisticPendingAccounts,
},
},
...newPersonalDetailsOnyxData.optimisticData,
];

const successData: OnyxUpdate[] = newPersonalDetailsOnyxData.finallyData;
const successData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
pendingAccounts: successPendingAccounts,
},
},
...newPersonalDetailsOnyxData.finallyData,
];

const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
participantAccountIDs: report.participantAccountIDs,
visibleChatMemberAccountIDs: report.visibleChatMemberAccountIDs,
pendingAccounts: failurePendingAccounts,
},
},
...newPersonalDetailsOnyxData.finallyData,
Expand All @@ -2339,17 +2360,27 @@ function inviteToRoom(reportID: string, inviteeEmailsToAccountIDs: Record<string
/** Removes people from a room */
function removeFromRoom(reportID: string, targetAccountIDs: number[]) {
const report = currentReportData?.[reportID];

const participantAccountIDsAfterRemoval = report?.participantAccountIDs?.filter((id: number) => !targetAccountIDs.includes(id));
const visibleChatMemberAccountIDsAfterRemoval = report?.visibleChatMemberAccountIDs?.filter((id: number) => !targetAccountIDs.includes(id));

const optimisticPendingAccounts: Record<string, PendingAccount> = {};
const successPendingAccounts: Record<string, PendingAccount> = {};
const failurePendingAccounts: Record<string, PendingAccount> = {};

targetAccountIDs.forEach((accountID) => {
optimisticPendingAccounts[accountID] = {pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE};
successPendingAccounts[accountID] = {pendingAction: null};
failurePendingAccounts[accountID] = {
errors: ErrorUtils.getMicroSecondOnyxError('report.people.error.genericRemove'),
};
});

const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
participantAccountIDs: participantAccountIDsAfterRemoval,
visibleChatMemberAccountIDs: visibleChatMemberAccountIDsAfterRemoval,
pendingAccounts: optimisticPendingAccounts,
},
},
];
Expand All @@ -2359,8 +2390,7 @@ function removeFromRoom(reportID: string, targetAccountIDs: number[]) {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
participantAccountIDs: report?.participantAccountIDs,
visibleChatMemberAccountIDs: report?.visibleChatMemberAccountIDs,
pendingAccounts: failurePendingAccounts,
},
},
];
Expand All @@ -2374,6 +2404,7 @@ function removeFromRoom(reportID: string, targetAccountIDs: number[]) {
value: {
participantAccountIDs: participantAccountIDsAfterRemoval,
visibleChatMemberAccountIDs: visibleChatMemberAccountIDsAfterRemoval,
pendingAccounts: successPendingAccounts,
},
},
];
Expand Down Expand Up @@ -2816,6 +2847,34 @@ function resolveActionableMentionWhisper(reportId: string, reportAction: OnyxEnt
API.write(WRITE_COMMANDS.RESOLVE_ACTIONABLE_MENTION_WHISPER, parameters, {optimisticData, failureData});
}

/**
* Removes an error after trying to delete a member
*/
function clearDeleteMemberError(reportID: string, accountID: number) {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
pendingAccounts: {
[accountID]: null,
},
});
}

/**
* Removes an error after trying to add a member
*/
function clearAddMemberError(reportID: string, accountID: number) {
const report = currentReportData?.[reportID];
const participantAccountIDs = report?.participantAccountIDs?.filter((id: number) => id !== accountID);
const visibleChatMemberAccountIDs = report?.visibleChatMemberAccountIDs?.filter((id: number) => id !== accountID);

Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
pendingAccounts: {
[accountID]: null,
},
participantAccountIDs,
visibleChatMemberAccountIDs,
});
}

export {
searchInServer,
addComment,
Expand Down Expand Up @@ -2885,4 +2944,6 @@ export {
updateReportField,
updateReportName,
resolveActionableMentionWhisper,
clearDeleteMemberError,
clearAddMemberError,
};
22 changes: 20 additions & 2 deletions src/pages/RoomMembersPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ function RoomMembersPage(props) {

const getMemberOptions = () => {
let result = [];
const pendingAccounts = props.report.pendingAccounts;

_.each(props.report.visibleChatMemberAccountIDs, (accountID) => {
const details = personalDetails[accountID];
Expand Down Expand Up @@ -205,7 +206,6 @@ function RoomMembersPage(props) {
return;
}
}

result.push({
keyForList: String(accountID),
accountID: Number(accountID),
Expand All @@ -220,14 +220,31 @@ function RoomMembersPage(props) {
type: CONST.ICON_TYPE_AVATAR,
},
],
pendingAction: _.get(pendingAccounts, [accountID, 'pendingAction']),
errors: _.get(pendingAccounts, [accountID, 'errors']),
});
});

result = _.sortBy(result, (value) => value.text.toLowerCase());

return result;
};

/**
* Dismisses the errors on one item
*
* @param {Object} item
*/
const dismissError = useCallback(
(item) => {
if (item.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) {
Report.clearDeleteMemberError(props.report.reportID, item.accountID);
} else {
Report.clearAddMemberError(props.report.reportID, item.accountID);
}
},
[props.report.reportID],
);

const isPolicyMember = useMemo(() => PolicyUtils.isPolicyMember(props.report.policyID, props.policies), [props.report.policyID, props.policies]);
const data = getMemberOptions();
const headerMessage = searchValue.trim() && !data.length ? props.translate('roomMembersPage.memberNotFound') : '';
Expand Down Expand Up @@ -291,6 +308,7 @@ function RoomMembersPage(props) {
showLoadingPlaceholder={!OptionsListUtils.isPersonalDetailsReady(personalDetails) || !didLoadRoomMembers}
showScrollIndicator
shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()}
onDismissError={dismissError}
/>
</View>
</View>
Expand Down
9 changes: 8 additions & 1 deletion src/types/onyx/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ type Participant = {

type Participants = Record<number, Participant>;

type PendingAccount = {
errors?: OnyxCommon.Errors;
pendingAction?: OnyxCommon.PendingAction | null;
};

type Report = {
/** The specific type of chat */
chatType?: ValueOf<typeof CONST.REPORT.CHAT_TYPE>;
Expand Down Expand Up @@ -172,8 +177,10 @@ type Report = {

/** If the report contains reportFields, save the field id and its value */
reportFields?: Record<string, PolicyReportField>;

pendingAccounts?: Record<string, PendingAccount>;
};

export default Report;

export type {NotificationPreference, WriteCapability, Note};
export type {NotificationPreference, WriteCapability, Note, PendingAccount};
Loading