Skip to content

Refactor the API for saving the report notification preferences #9501

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.

Sign up for GitHub

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

Merged
marcaaron merged 10 commits into from
Jul 6, 2022
Merged

Refactor the API for saving the report notification preferences #9501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c8fb89d
Remove the old API method
tgolen Jun 20, 2022
8572e2b
Replace string with constant
tgolen Jun 20, 2022
b6bfb37
Merge branch 'main' into tgolen-report-notifications
tgolen Jun 21, 2022
cec863d
Set failure data to revert the notification preference
tgolen Jun 21, 2022
7cb3aad
Use a constant for onyx methods
tgolen Jun 21, 2022
dd1519b
Merge branch 'main' into tgolen-report-notifications
tgolen Jun 22, 2022
5bb3243
Merge branch 'main' into tgolen-report-notifications
tgolen Jul 4, 2022
882d50f
Add missing JS docs
tgolen Jul 5, 2022
71af858
Merge remote-tracking branch 'origin/tgolen-report-notifications' int…
tgolen Jul 5, 2022
841baf3
Update src/libs/actions/Report.js
tgolen Jul 6, 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
6 changes: 6 additions & 0 deletions src/CONST.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,12 @@ const CONST = {

// There's a limit of 60k characters in Auth - https://github.com/Expensify/Auth/blob/198d59547f71fdee8121325e8bc9241fc9c3236a/auth/lib/Request.h#L28
MAX_COMMENT_LENGTH: 60000,

ONYX: {
METHOD: {
MERGE: 'merge',
},
},
};

export default CONST;
26 changes: 19 additions & 7 deletions src/libs/actions/Report.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ function getSimplifiedReportObject(report) {
const oldPolicyName = lodashGet(report, ['reportNameValuePairs', 'oldPolicyName'], '').toString();

const lastActorEmail = lodashGet(report, 'lastActionActorEmail', '');
const notificationPreference = lodashGet(report, ['reportNameValuePairs', 'notificationPreferences', currentUserAccountID], 'daily');
const notificationPreference = lodashGet(report, ['reportNameValuePairs', 'notificationPreferences', currentUserAccountID], CONST.REPORT.NOTIFICATION_PREFERENCE.DAILY);

// Used for User Created Policy Rooms, will denote how access to a chat room is given among workspace members
const visibility = lodashGet(report, ['reportNameValuePairs', 'visibility']);
Expand Down Expand Up @@ -1312,14 +1312,26 @@ function syncChatAndIOUReports(chatReport, iouReport) {
}

/**
* Updates a user's notification preferences for a chat room
*
* @param {Number} reportID
* @param {String} notificationPreference
* @param {String} previousValue
* @param {String} newValue
*/
function updateNotificationPreference(reportID, notificationPreference) {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {notificationPreference});
DeprecatedAPI.Report_UpdateNotificationPreference({reportID, notificationPreference});
function updateNotificationPreference(reportID, previousValue, newValue) {

@marcaaron marcaaron Jul 5, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we are missing JSDocs for this.

const optimisticData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {notificationPreference: newValue},
},
];
const failureData = [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {notificationPreference: previousValue},
},
];
API.write('UpdateReportNotificationPreference', {reportID, notificationPreference: newValue}, {optimisticData, failureData});
}

/**
Expand Down
14 changes: 0 additions & 14 deletions src/libs/deprecatedAPI.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,19 +368,6 @@ function Report_UpdateLastRead(parameters) {
return Network.post(commandName, parameters);
}

/**
* @param {Object} parameters
* @param {Number} parameters.reportID
* @param {String} parameters.notificationPreference
* @returns {Promise}
*
*/
function Report_UpdateNotificationPreference(parameters) {
const commandName = 'Report_UpdateNotificationPreference';
requireParameters(['reportID', 'notificationPreference'], parameters, commandName);
return Network.post(commandName, parameters);
}

/**
* @param {Object} parameters
* @param {String} parameters.email
Expand Down Expand Up @@ -956,7 +943,6 @@ export {
Report_TogglePinned,
Report_EditComment,
Report_UpdateLastRead,
Report_UpdateNotificationPreference,
ResendValidateCode,
ResetPassword,
SetNameValuePair,
Expand Down
1 change: 1 addition & 0 deletions src/pages/ReportSettingsPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ class ReportSettingsPage extends Component {
onInputChange={(notificationPreference) => {
Report.updateNotificationPreference(
this.props.report.reportID,
this.props.report.notificationPreference,
notificationPreference,
);
}}
Expand Down