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

Remove deprecated createOrFetchReport method #12157

Merged
merged 26 commits into from
Nov 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
6ed4868
Just navigate to the concierge chat right away since it will always e…
stitesExpensify Oct 26, 2022
b49f4de
Move navigating or creating logic to report actions
stitesExpensify Oct 26, 2022
e90b58c
Just navigate to the concierge chat right away since it will always e…
stitesExpensify Oct 26, 2022
e8b3448
Remove uses of fetchOrCreateReport
stitesExpensify Oct 26, 2022
7bccf56
Remove unused fetchAllReports method
stitesExpensify Oct 26, 2022
17bda6f
Remove deprecated fetchOrCreateChatReport method
stitesExpensify Oct 26, 2022
3a7c17c
Switch to navigate method
stitesExpensify Oct 26, 2022
9ff5456
remove fetchChatReportsByIDs
stitesExpensify Oct 26, 2022
1ef2f2e
Fix comment
stitesExpensify Oct 27, 2022
977b22f
Remove unused method fetchIOUReportID
stitesExpensify Oct 27, 2022
1417e8f
Remove unused imports
stitesExpensify Oct 27, 2022
adaa619
Update name to better represent action taken
stitesExpensify Oct 27, 2022
02f5ed6
Merge branch 'main' of github.com:Expensify/App into stites-removeCre…
stitesExpensify Nov 2, 2022
95e3e8d
Remove unused method
stitesExpensify Nov 2, 2022
0a8b57b
Fix documentation since fetchOrCreate has been removed
stitesExpensify Nov 2, 2022
93a778f
Remove timing because it is no longer being ended
stitesExpensify Nov 2, 2022
bf8f0c7
Remove unused constants
stitesExpensify Nov 2, 2022
4e14e18
remove unused deprecated method
stitesExpensify Nov 3, 2022
89f24b5
Add comment
stitesExpensify Nov 3, 2022
fabe56d
Merge branch 'main' of github.com:Expensify/App into stites-removeCre…
stitesExpensify Nov 3, 2022
8587f79
Merge branch 'main' of github.com:Expensify/App into stites-removeCre…
stitesExpensify Nov 3, 2022
92405ee
Merge branch 'main' of github.com:Expensify/App into stites-removeCre…
stitesExpensify Nov 4, 2022
dc8f2c5
Merge branch 'main' of github.com:Expensify/App into stites-removeCre…
stitesExpensify Nov 7, 2022
486fa63
Add default chat report name to const
stitesExpensify Nov 7, 2022
b1912a8
Always add a default room name
stitesExpensify Nov 7, 2022
d827fc4
Remove unused file
stitesExpensify Nov 9, 2022
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ This application is built with the following principles.
- The UI should never call any Onyx methods except for `Onyx.connect()`. That is the job of Actions (see next section).
- The UI always triggers an Action when something needs to happen (eg. a person inputs data, the UI triggers an Action with this data).
- The UI should be as flexible as possible when it comes to:
- Incomplete or missing data. Always assume data is incomplete or not there. For example, when a comment is pushed to the client from a pusher event, it's possible that Onyx does not have data for that report yet. That's OK. A partial report object is added to Onyx for the report key `report_1234 = {reportID: 1234, isUnread: true}`. Then there is code that monitors Onyx for reports with incomplete data, and calls `fetchChatReportsByIDs(1234)` to get the full data for that report. The UI should be able to gracefully handle the report object not being complete. In this example, the sidebar wouldn't display any report that does not have a report name.
- Incomplete or missing data. Always assume data is incomplete or not there. For example, when a comment is pushed to the client from a pusher event, it's possible that Onyx does not have data for that report yet. That's OK. A partial report object is added to Onyx for the report key `report_1234 = {reportID: 1234, isUnread: true}`. Then there is code that monitors Onyx for reports with incomplete data, and calls `openReport(1234)` to get the full data for that report. The UI should be able to gracefully handle the report object not being complete. In this example, the sidebar wouldn't display any report that does not have a report name.
- The order that actions are done in. All actions should be done in parallel instead of sequence.
- Parallel actions are asynchronous methods that don't return promises. Any number of these actions can be called at one time and it doesn't matter what order they happen in or when they complete.
- In-Sequence actions are asynchronous methods that return promises. This is necessary when one asynchronous method depends on the results from a previous asynchronous method. Example: Making an XHR to `command=CreateChatReport` which returns a reportID which is used to call `command=Get&rvl=reportStuff`.
Expand Down
5 changes: 1 addition & 4 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,6 @@ const CONST = {
REMOVED_FROM_POLICY: 'removedFromPolicy',
POLICY_DELETED: 'policyDeleted',
},
ERROR: {
INACCESSIBLE_REPORT: 'Report not found',
},
MESSAGE: {
TYPE: {
COMMENT: 'COMMENT',
Expand Down Expand Up @@ -318,6 +315,7 @@ const CONST = {
MAX_ROOM_NAME_LENGTH: 80,
LAST_MESSAGE_TEXT_MAX_LENGTH: 200,
OWNER_EMAIL_FAKE: '__FAKE__',
DEFAULT_REPORT_NAME: 'Chat Report',
},
COMPOSER: {
MAX_LINES: 16,
Expand Down Expand Up @@ -351,7 +349,6 @@ const CONST = {
SEARCH_RENDER: 'search_render',
HOMEPAGE_INITIAL_RENDER: 'homepage_initial_render',
REPORT_INITIAL_RENDER: 'report_initial_render',
HOMEPAGE_REPORTS_LOADED: 'homepage_reports_loaded',
SWITCH_REPORT: 'switch_report',
SIDEBAR_LOADED: 'sidebar_loaded',
COLD: 'cold',
Expand Down
1 change: 0 additions & 1 deletion src/libs/Navigation/AppNavigator/AuthScreens.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ class AuthScreens extends React.Component {
super(props);

Timing.start(CONST.TIMING.HOMEPAGE_INITIAL_RENDER);
Timing.start(CONST.TIMING.HOMEPAGE_REPORTS_LOADED);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This timer was being started but never stopped / recorded, because the stop for it was in fetchAllReports, which was no longer being called anywhere.

}

componentDidMount() {
Expand Down
2 changes: 1 addition & 1 deletion src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ function buildOptimisticIOUReportAction(sequenceNumber, type, amount, currency,
*/
function buildOptimisticChatReport(
participantList,
reportName = 'Chat Report',
reportName = CONST.REPORT.DEFAULT_REPORT_NAME,
chatType = '',
policyID = CONST.POLICY.OWNER_EMAIL_FAKE,
ownerEmail = CONST.REPORT.OWNER_EMAIL_FAKE,
Expand Down
231 changes: 25 additions & 206 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@ import ONYXKEYS from '../../ONYXKEYS';
import * as Pusher from '../Pusher/pusher';
import LocalNotification from '../Notification/LocalNotification';
import PushNotification from '../Notification/PushNotification';
import * as PersonalDetails from './PersonalDetails';
import Navigation from '../Navigation/Navigation';
import * as ActiveClientManager from '../ActiveClientManager';
import Visibility from '../Visibility';
import ROUTES from '../../ROUTES';
import Timing from './Timing';
import * as DeprecatedAPI from '../deprecatedAPI';
import * as API from '../API';
import CONFIG from '../../CONFIG';
Expand All @@ -25,6 +23,7 @@ import Growl from '../Growl';
import * as Localize from '../Localize';
import DateUtils from '../DateUtils';
import * as ReportActionsUtils from '../ReportActionsUtils';
import * as OptionsListUtils from '../OptionsListUtils';

let currentUserEmail;
let currentUserAccountID;
Expand Down Expand Up @@ -216,118 +215,6 @@ function fetchIOUReport(iouReportID, chatReportID) {
});
}

/**
* Given debtorEmail finds active IOU report ID via GetIOUReport API call
*
* @param {String} debtorEmail
* @returns {Promise}
*/
function fetchIOUReportID(debtorEmail) {
return DeprecatedAPI.GetIOUReport({
debtorEmail,
}).then((response) => {
const iouReportID = response.reportID || 0;
if (response.jsonCode !== 200) {
console.error(response.message);
return;
}
if (iouReportID === 0) {
// If there is no IOU report for this user then we will assume it has been paid and do nothing here.
// All reports are initialized with hasOutstandingIOU: false. Since the IOU report we were looking for has
// been settled then there's nothing more to do.
Log.info('GetIOUReport returned a reportID of 0, not fetching IOU report data');
return;
}
return iouReportID;
});
}

/**
* Fetches chat reports when provided a list of chat report IDs.
* If the shouldRedirectIfInaccessible flag is set, we redirect to the Concierge chat
* when we find an inaccessible chat
* @param {Array} chatList
* @param {Boolean} shouldRedirectIfInaccessible
* @returns {Promise<Object[]>} only used internally when fetchAllReports() is called
*/
function fetchChatReportsByIDs(chatList, shouldRedirectIfInaccessible = false) {
let fetchedReports;
const simplifiedReports = {};
return DeprecatedAPI.GetReportSummaryList({reportIDList: chatList.join(',')})
.then(({reportSummaryList, jsonCode}) => {
Log.info('[Report] successfully fetched report data', false, {chatList});
fetchedReports = reportSummaryList;

// If we receive a 404 response while fetching a single report, treat that report as inaccessible.
if (jsonCode === 404 && shouldRedirectIfInaccessible) {
throw new Error(CONST.REPORT.ERROR.INACCESSIBLE_REPORT);
}

return Promise.all(_.map(fetchedReports, (chatReport) => {
// If there aren't any IOU actions, we don't need to fetch any additional data
if (!chatReport.hasIOUAction) {
return;
}

// Group chat reports cannot and should not be associated with a specific IOU report
const participants = getParticipantEmailsFromReport(chatReport);
if (participants.length > 1) {
return;
}
if (participants.length === 0) {
Log.alert('[Report] Report with IOU action but does not have any participant.', {
reportID: chatReport.reportID,
participants,
});
return;
}

return fetchIOUReportID(participants[0])
.then((iouReportID) => {
if (!iouReportID) {
return Promise.resolve();
}

return fetchIOUReport(iouReportID, chatReport.reportID);
});
}));
})
.then((iouReportObjects) => {
// Process the reports and store them in Onyx. At the same time we'll save the simplified reports in this
// variable called simplifiedReports which hold the participants (minus the current user) for each report.
// Using this simplifiedReport we can call PersonalDetails.getFromReportParticipants to get the
// personal details of all the participants and even link up their avatars to report icons.
const reportIOUData = {};
_.each(fetchedReports, (report) => {
const simplifiedReport = getSimplifiedReportObject(report);
simplifiedReports[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = simplifiedReport;
});

_.each(iouReportObjects, (iouReportObject) => {
if (!iouReportObject) {
return;
}

const iouReportKey = `${ONYXKEYS.COLLECTION.REPORT_IOUS}${iouReportObject.reportID}`;
const reportKey = `${ONYXKEYS.COLLECTION.REPORT}${iouReportObject.chatReportID}`;
reportIOUData[iouReportKey] = iouReportObject;
simplifiedReports[reportKey].iouReportID = iouReportObject.reportID;
simplifiedReports[reportKey].hasOutstandingIOU = iouReportObject.stateNum
Comment on lines -314 to -315
Copy link
Contributor

Choose a reason for hiding this comment

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

Good riddance simplifiedReports 👋

=== CONST.REPORT.STATE_NUM.PROCESSING && iouReportObject.total !== 0;
});

// We use mergeCollection such that it updates the collection in one go.
// Any withOnyx subscribers to this key will also receive the complete updated props just once
// than updating props for each report and re-rendering had merge been used.
Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT_IOUS, reportIOUData);
Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, simplifiedReports);

// Fetch the personal details if there are any
PersonalDetails.getFromReportParticipants(_.values(simplifiedReports));
return simplifiedReports;
});
}

/**
* Given IOU object, save the data to Onyx.
*
Expand Down Expand Up @@ -481,91 +368,6 @@ function unsubscribeFromReportChannel(reportID) {
Pusher.unsubscribe(pusherChannelName);
}

/**
* Get the report ID for a chat report for a specific
* set of participants and navigate to it if wanted.
*
* @param {String[]} participants
* @param {Boolean} shouldNavigate
* @returns {Promise<Object[]>}
*/
function fetchOrCreateChatReport(participants, shouldNavigate = true) {
if (participants.length < 2) {
throw new Error('fetchOrCreateChatReport() must have at least two participants.');
}

return DeprecatedAPI.CreateChatReport({
emailList: participants.join(','),
})
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
.then((data) => {
if (data.jsonCode !== 200) {
console.error(data.message);
Growl.error(data.message);
return;
}

// Merge report into Onyx
const simplifiedReportObject = getSimplifiedReportObject(data);
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${data.reportID}`, simplifiedReportObject);

// Fetch the personal details if there are any
PersonalDetails.getFromReportParticipants([simplifiedReportObject]);

if (shouldNavigate) {
// Redirect the logged in person to the new report
Navigation.navigate(ROUTES.getReportRoute(data.reportID));
}

// We are returning an array with a report object here since fetchAllReports calls this method or
// fetchChatReportsByIDs which returns an array of report objects.
return [simplifiedReportObject];
});
}

/**
* Get all of our reports
*
* @param {Boolean} shouldRecordHomePageTiming whether or not performance timing should be measured
* @returns {Promise}
*/
function fetchAllReports(
shouldRecordHomePageTiming = false,
) {
Onyx.set(ONYXKEYS.IS_LOADING_REPORT_DATA, true);
return DeprecatedAPI.Get({
returnValueList: 'chatList',
})
.then((response) => {
if (response.jsonCode !== 200) {
return;
}

// The cast here is necessary as Get rvl='chatList' may return an int or Array
const reportIDs = _.filter(String(response.chatList).split(','), _.identity);

// Get all the chat reports if they have any, otherwise create one with concierge
if (reportIDs.length > 0) {
return fetchChatReportsByIDs(reportIDs);
}

return fetchOrCreateChatReport([currentUserEmail, CONST.EMAIL.CONCIERGE], false);
})
.then((returnedReports) => {
Onyx.set(ONYXKEYS.IS_LOADING_REPORT_DATA, false);

// If at this point the user still doesn't have a Concierge report, create it for them.
// This means they were a participant in reports before their account was created (e.g. default rooms)
const hasConciergeChat = _.some(returnedReports, report => ReportUtils.isConciergeChatReport(report));
if (!hasConciergeChat) {
fetchOrCreateChatReport([currentUserEmail, CONST.EMAIL.CONCIERGE], false);
}

if (shouldRecordHomePageTiming) {
Timing.end(CONST.TIMING.HOMEPAGE_REPORTS_LOADED);
}
});
}

/**
* Add up to two report actions to a report. This method can be called for the following situations:
*
Expand Down Expand Up @@ -708,6 +510,7 @@ function openReport(reportID, participantList = [], newReportObject = {}) {
isLoadingMoreReportActions: false,
lastVisitedTimestamp: Date.now(),
lastReadSequenceNumber: getMaxSequenceNumber(reportID),
reportName: CONST.REPORT.DEFAULT_REPORT_NAME,
Copy link
Contributor

Choose a reason for hiding this comment

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

This caused a regression. Whenever a report is opened the name is reset to the default, and it's only set back to the proper name, like #admins, when the response comes back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for letting me know and taking care of it!

},
}],
successData: [{
Expand Down Expand Up @@ -756,6 +559,25 @@ function openReport(reportID, participantList = [], newReportObject = {}) {
onyxData);
}

/**
* This will find an existing chat, or create a new one if none exists, for the given user or set of users. It will then navigate to this chat.
*
* @param {Array} userLogins list of user logins.
*/
function navigateToAndOpenReport(userLogins) {
const formattedUserLogins = _.map(userLogins, login => OptionsListUtils.addSMSDomainIfPhoneNumber(login).toLowerCase());
let newChat = {};
const chat = ReportUtils.getChatByParticipants(formattedUserLogins);
if (!chat) {
newChat = ReportUtils.buildOptimisticChatReport(formattedUserLogins);
}
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
const reportID = chat ? chat.reportID : newChat.reportID;

// We want to pass newChat here because if anything is passed in that param (even an existing chat), we will try to create a chat on the server
openReport(reportID, newChat.participants, newChat);
Navigation.navigate(ROUTES.getReportRoute(reportID));
}

/**
* Get the latest report history without marking the report as read.
*
Expand Down Expand Up @@ -969,7 +791,7 @@ function handleReportChanged(report) {
// A report can be missing a name if a comment is received via pusher event
// and the report does not yet exist in Onyx (eg. a new DM created with the logged in person)
if (report.reportID && report.reportName === undefined) {
fetchChatReportsByIDs([report.reportID]);
openReport(report.reportID);
Copy link
Member

Choose a reason for hiding this comment

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

Calling OpenReport here caused a regression #13913 . OpenReport also marks the report as read.

}
}

Expand Down Expand Up @@ -1218,7 +1040,7 @@ function updateNotificationPreference(reportID, previousValue, newValue) {
function navigateToConciergeChat() {
// If we don't have a chat with Concierge then create it
if (!conciergeChatReportID) {
fetchOrCreateChatReport([currentUserEmail, CONST.EMAIL.CONCIERGE], true);
navigateToAndOpenReport([CONST.EMAIL.CONCIERGE]);
return;
}

Expand Down Expand Up @@ -1250,8 +1072,7 @@ function addPolicyReport(policy, reportName, visibility) {
);

// Onyx.set is used on the optimistic data so that it is present before navigating to the workspace room. With Onyx.merge the workspace room reportID is not present when
// fetchReportIfNeeded is called on the ReportScreen, so fetchChatReportsByIDs is called which is unnecessary since the optimistic data will be stored in Onyx.
// If there was an error creating the room, then fetchChatReportsByIDs throws an error and the user is navigated away from the report instead of showing the RBR error message.
// fetchReportIfNeeded is called on the ReportScreen, so openReport is called which is unnecessary since the optimistic data will be stored in Onyx.
// Therefore, Onyx.set is used instead of Onyx.merge.
const optimisticData = [
{
Expand Down Expand Up @@ -1521,9 +1342,6 @@ Onyx.connect({
});

export {
fetchAllReports,
fetchOrCreateChatReport,
fetchChatReportsByIDs,
fetchIOUReportByID,
stitesExpensify marked this conversation as resolved.
Show resolved Hide resolved
addComment,
addAttachment,
Expand All @@ -1549,6 +1367,7 @@ export {
readNewestAction,
readOldestAction,
openReport,
navigateToAndOpenReport,
openPaymentDetailsPage,
updatePolicyRoomName,
clearPolicyRoomNameErrors,
Expand Down
Loading