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 unused report functions #12357

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
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
4 changes: 0 additions & 4 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,6 @@ const CONST = {
REMOVED_FROM_POLICY: 'removedFromPolicy',
POLICY_DELETED: 'policyDeleted',
},
ERROR: {
INACCESSIBLE_REPORT: 'Report not found',
},
MESSAGE: {
TYPE: {
COMMENT: 'COMMENT',
Expand Down Expand Up @@ -348,7 +345,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
172 changes: 4 additions & 168 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ 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 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
=== 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 @@ -487,14 +374,13 @@ function unsubscribeFromReportChannel(reportID) {
*
* @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({
DeprecatedAPI.CreateChatReport({
emailList: participants.join(','),
})
.then((data) => {
Expand All @@ -515,54 +401,6 @@ function fetchOrCreateChatReport(participants, shouldNavigate = true) {
// 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);
}
});
}

Expand Down Expand Up @@ -969,7 +807,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
Contributor Author

Choose a reason for hiding this comment

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

This is the only risky change – need to make extra sure that openReport will do everything that fetchChatReportsByIDs once did

}
}

Expand Down Expand Up @@ -1250,8 +1088,8 @@ 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.
// If there was an error creating the room, then openReport throws an error and the user is navigated away from the report instead of showing the RBR error message.
// Therefore, Onyx.set is used instead of Onyx.merge.
const optimisticData = [
{
Expand Down Expand Up @@ -1503,9 +1341,7 @@ Onyx.connect({
});

export {
fetchAllReports,
fetchOrCreateChatReport,
fetchChatReportsByIDs,
fetchIOUReportByID,
addComment,
addAttachment,
Expand Down
12 changes: 0 additions & 12 deletions src/libs/deprecatedAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -481,17 +481,6 @@ function Inbox_CallUser(parameters) {
return Network.post(commandName, parameters);
}

/**
* @param {Object} parameters
* @param {String} parameters.reportIDList
* @returns {Promise}
*/
function GetReportSummaryList(parameters) {
const commandName = 'Get';
requireParameters(['reportIDList'], parameters, commandName);
return Network.post(commandName, {...parameters, returnValueList: 'reportSummaryList'});
}

/**
* Transfer Wallet balance and takes either the bankAccoundID or fundID
* @param {Object} parameters
Expand Down Expand Up @@ -529,7 +518,6 @@ export {
GetIOUReport,
GetFullPolicy,
GetPolicySummaryList,
GetReportSummaryList,
Graphite_Timer,
Inbox_CallUser,
PayIOU,
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class ReportScreen extends React.Component {
return;
}

Report.fetchChatReportsByIDs([reportIDFromPath], true);
Report.openReport(reportIDFromPath);
}

/**
Expand Down