Fixed: billing account was reset in UI after reopening working period's details.

Author: MadOPcode

src/routes/WorkPeriods/components/PeriodDetails/index.jsx

@@ -26,15 +26,19 @@ import styles from "./styles.module.scss";
  * @param {Object} props.details working period details object
  * @param {boolean} props.isDisabled whether the details are disabled
  * @param {boolean} props.isFailed whether the payments for the period has failed
+ * @param {Object} props.period working period basic data object
  * @returns {JSX.Element}
  */
-const PeriodDetails = ({ className, details, isDisabled, isFailed }) => {
+const PeriodDetails = ({
+  className,
+  details,
+  isDisabled,
+  isFailed,
+  period,
+}) => {
   const dispatch = useDispatch();
+  const { id: periodId, rbId, jobId, billingAccountId } = period;
   const {
-    periodId,
-    rbId,
-    jobId,
-    billingAccountId,
     billingAccounts,
     billingAccountsError,
     billingAccountsIsDisabled,
@@ -153,10 +157,6 @@ const PeriodDetails = ({ className, details, isDisabled, isFailed }) => {
 PeriodDetails.propTypes = {
   className: PT.string,
   details: PT.shape({
-    periodId: PT.string.isRequired,
-    rbId: PT.string.isRequired,
-    jobId: PT.string.isRequired,
-    billingAccountId: PT.number.isRequired,
     billingAccounts: PT.arrayOf(
       PT.shape({
         label: PT.string.isRequired,
@@ -172,6 +172,12 @@ PeriodDetails.propTypes = {
   }).isRequired,
   isDisabled: PT.bool.isRequired,
   isFailed: PT.bool.isRequired,
+  period: PT.shape({
+    id: PT.string.isRequired,
+    rbId: PT.string.isRequired,
+    jobId: PT.string.isRequired,
+    billingAccountId: PT.number.isRequired,
+  }).isRequired,
 };
 
 export default memo(PeriodDetails);

src/routes/WorkPeriods/components/PeriodItem/index.jsx

@@ -214,6 +214,7 @@ const PeriodItem = ({
       {details && (
         <PeriodDetails
           className={styles.periodDetails}
+          period={item}
           details={details}
           isDisabled={isDisabled}
           isFailed={!!reasonFailed}
@@ -252,6 +253,7 @@ PeriodItem.propTypes = {
     id: PT.oneOfType([PT.number, PT.string]).isRequired,
     jobId: PT.string,
     rbId: PT.string.isRequired,
+    billingAccountId: PT.number.isRequired,
     projectId: PT.oneOfType([PT.number, PT.string]).isRequired,
     userHandle: PT.string.isRequired,
     teamName: PT.oneOfType([PT.number, PT.string]).isRequired,
@@ -268,10 +270,6 @@ PeriodItem.propTypes = {
     paymentTotal: PT.number.isRequired,
   }).isRequired,
   details: PT.shape({
-    periodId: PT.string.isRequired,
-    rbId: PT.string.isRequired,
-    jobId: PT.string.isRequired,
-    billingAccountId: PT.number.isRequired,
     billingAccounts: PT.arrayOf(
       PT.shape({
         label: PT.string.isRequired,

src/store/actions/workPeriods.js

@@ -92,25 +92,25 @@ export const loadWorkPeriodDetailsError = (periodId, message) => ({
 /**
  * Creates an action denoting successful load of billing accounts.
  *
- * @param {string} periodId working period id
+ * @param {Object} period working period basic data object
  * @param {Array} accounts billing accounts
  * @returns {Object}
  */
-export const loadBillingAccountsSuccess = (periodId, accounts) => ({
+export const loadBillingAccountsSuccess = (period, accounts) => ({
   type: ACTION_TYPE.WP_LOAD_BILLING_ACCOUNTS_SUCCESS,
-  payload: { periodId, accounts },
+  payload: { period, accounts },
 });
 
 /**
  * Creates an action denoting an error while loading billing accounts.
  *
- * @param {string} periodId working period id
+ * @param {Object} period working period basic data object
  * @param {string} message error message
  * @returns {Object}
  */
-export const loadBillingAccountsError = (periodId, message) => ({
+export const loadBillingAccountsError = (period, message) => ({
   type: ACTION_TYPE.WP_LOAD_BILLING_ACCOUNTS_ERROR,
-  payload: { periodId, message, id: nextErrorId++ },
+  payload: { period, message, id: nextErrorId++ },
 });
 
 /**

src/store/reducers/workPeriods.js

@@ -49,14 +49,13 @@ const initPeriodData = (period) => {
   return data;
 };
 
-const initPeriodDetails = (period, cancelSource = cancelSourceDummy) => ({
-  periodId: period.id,
-  rbId: period.rbId,
+const initPeriodDetails = (
+  billingAccountId = 0,
+  cancelSource = cancelSourceDummy
+) => ({
   cancelSource,
-  jobId: period.jobId,
-  billingAccountId: period.billingAccountId || 0,
   billingAccounts: [
-    { value: period.billingAccountId || 0, label: BILLING_ACCOUNTS_LOADING },
+    { value: billingAccountId, label: BILLING_ACCOUNTS_LOADING },
   ],
   billingAccountsError: null,
   billingAccountsIsDisabled: true,
@@ -191,7 +190,10 @@ const actionHandlers = {
     { period, cancelSource }
   ) => {
     const periodsDetails = { ...state.periodsDetails };
-    periodsDetails[period.id] = initPeriodDetails(period, cancelSource);
+    periodsDetails[period.id] = initPeriodDetails(
+      period.billingAccountId,
+      cancelSource
+    );
     return {
       ...state,
       periodsDetails,
@@ -201,7 +203,7 @@ const actionHandlers = {
     state,
     { periodId, details }
   ) => {
-    const periodsDetails = { ...state.periodsDetails };
+    const periodsDetails = state.periodsDetails;
     let periodDetails = periodsDetails[periodId];
     // period details object must already be initialized
     if (!periodDetails) {
@@ -232,7 +234,7 @@ const actionHandlers = {
     return {
       ...state,
       periodsData: [periodsData],
-      periodsDetails,
+      periodsDetails: { ...periodsDetails },
     };
   },
   [ACTION_TYPE.WP_LOAD_PERIOD_DETAILS_ERROR]: (
@@ -250,16 +252,26 @@ const actionHandlers = {
   },
   [ACTION_TYPE.WP_LOAD_BILLING_ACCOUNTS_SUCCESS]: (
     state,
-    { periodId, accounts }
+    { period, accounts }
   ) => {
-    const periodsDetails = { ...state.periodsDetails };
-    let periodDetails = periodsDetails[periodId];
+    const periodsDetails = state.periodsDetails;
+    let periodDetails = periodsDetails[period.id];
     if (!periodDetails) {
       // Period details may be removed at this point so we must handle this case.
       return state;
     }
+    let accountId = period.billingAccountId;
+    let hasAssignedAccount = false;
+    for (let account of accounts) {
+      if (account.value === accountId) {
+        hasAssignedAccount = true;
+        break;
+      }
+    }
+    if (accountId > 0 && !hasAssignedAccount) {
+      accounts.unshift(createAssignedBillingAccountOption(accountId));
+    }
     let billingAccountsIsDisabled = false;
-    let accountId = periodDetails.billingAccountId;
     if (!accounts.length) {
       accounts.push({ value: accountId, label: BILLING_ACCOUNTS_NONE });
       billingAccountsIsDisabled = true;
@@ -274,24 +286,24 @@ const actionHandlers = {
     if (!periodDetails.periodsIsLoading) {
       periodDetails.cancelSource = null;
     }
-    periodsDetails[periodId] = periodDetails;
+    periodsDetails[period.id] = periodDetails;
     return {
       ...state,
-      periodsDetails,
+      periodsDetails: { ...periodsDetails },
     };
   },
   [ACTION_TYPE.WP_LOAD_BILLING_ACCOUNTS_ERROR]: (
     state,
-    { periodId, message }
+    { period, message }
   ) => {
-    const periodsDetails = { ...state.periodsDetails };
-    let periodDetails = periodsDetails[periodId];
+    const periodsDetails = state.periodsDetails;
+    let periodDetails = periodsDetails[period.id];
     if (!periodDetails) {
       return state;
     }
     let billingAccounts = [];
     let billingAccountsIsDisabled = true;
-    let accountId = periodDetails.billingAccountId;
+    let accountId = period.billingAccountId;
     if (accountId) {
       billingAccounts.push(createAssignedBillingAccountOption(accountId));
       billingAccountsIsDisabled = false;
@@ -308,27 +320,26 @@ const actionHandlers = {
     if (!periodDetails.periodsIsLoading) {
       periodDetails.cancelSource = null;
     }
-    periodsDetails[periodId] = periodDetails;
+    periodsDetails[period.id] = periodDetails;
     return {
       ...state,
-      periodsDetails,
+      periodsDetails: { ...periodsDetails },
     };
   },
   [ACTION_TYPE.WP_SET_BILLING_ACCOUNT]: (state, { periodId, accountId }) => {
-    let periodsDetails = state.periodsDetails;
-    const periodDetails = periodsDetails[periodId];
-    if (!periodDetails) {
-      return state;
+    const periods = state.periods;
+    for (let i = 0, len = periods.length; i < len; i++) {
+      let period = periods[i];
+      if (period.id === periodId) {
+        periods[i] = { ...period, billingAccountId: accountId };
+        break;
+      }
     }
-    periodsDetails[periodId] = {
-      ...periodDetails,
-      billingAccountId: accountId,
-    };
-    periodsDetails = { ...periodsDetails };
     state = {
       ...state,
-      periodsDetails,
+      periods: [...periods],
     };
+    // updating reasons for which the period's selection may be disabled
     const periodsDisabledMap = state.periodsDisabled[0];
     const oldReasonsDisabled = periodsDisabledMap.get(periodId);
     const reasonsDisabled = removeReasonDisabled(

src/store/thunks/workPeriods.js

@@ -156,18 +156,13 @@ export const toggleWorkPeriodDetails =
         );
         bilAccsPromise
           .then((data) => {
-            const periodsDetails = selectors.getWorkPeriodsDetails(getState());
-            const periodDetails = periodsDetails[period.id];
-            const billingAccountId =
-              (periodDetails && periodDetails.billingAccountId) ||
-              period.billingAccountId;
-            const accounts = normalizeBillingAccounts(data, billingAccountId);
-            dispatch(actions.loadBillingAccountsSuccess(period.id, accounts));
+            const accounts = normalizeBillingAccounts(data);
+            dispatch(actions.loadBillingAccountsSuccess(period, accounts));
           })
           .catch((error) => {
             if (!axios.isCancel(error)) {
               dispatch(
-                actions.loadBillingAccountsError(period.id, error.toString())
+                actions.loadBillingAccountsError(period, error.toString())
               );
             }
           });

src/utils/workPeriods.js

@@ -31,6 +31,8 @@ export function findReasonsDisabled(period) {
   return reasons.length ? reasons : undefined;
 }
 
+export function createAlerts(period, bookingEndDate) {}
+
 export function addReasonDisabled(reasons, reason) {
   if (!reasons) {
     return [reason];
@@ -171,15 +173,12 @@ export function normalizePaymentStatus(paymentStatus) {
  * billing account.
  *
  * @param {Array} accounts array of billing accounts received for specific project
- * @param {number} accountId resource booking's billing account id
  * @returns {Array}
  */
-export function normalizeBillingAccounts(accounts, accountId = -1) {
+export function normalizeBillingAccounts(accounts) {
   const accs = [];
-  let hasSelectedAccount = false;
   for (let acc of accounts) {
     const value = +acc.tcBillingAccountId;
-    hasSelectedAccount = hasSelectedAccount || value === accountId;
     const endDate = acc.endDate
       ? moment(acc.endDate).format("DD MMM YYYY")
       : "";
@@ -188,9 +187,6 @@ export function normalizeBillingAccounts(accounts, accountId = -1) {
       label: `${acc.name} (${value})` + (endDate ? ` - ${endDate}` : ""),
     });
   }
-  if (!hasSelectedAccount && accountId > 0) {
-    accs.unshift(createAssignedBillingAccountOption(accountId));
-  }
   return accs;
 }