Refactored: setEcomStore and setShopifyConfig actions to accept ID. (#2tzh44d) #241

alsoK2maan · 2022-09-20T10:03:46Z

Related Issues

Closes #

Short Description and Why It's Useful

Refactored action setEcomStore and setShopifyConfig to accept ID as well as object, as initially the whole object was passed, posing unnecessary code.

Screenshots of Visual Changes before/after (If There Are Any)

IMPORTANT NOTICE - Remember to add changelog entry

Contribution and Currently Important Rules Acceptance

I read and followed contribution rules

…date and time, instead of using minDateTime

…t ID as parameter (##2tzh44d)

adityasharma7 · 2022-09-23T12:58:37Z

src/views/Settings.vue

-        this.store.dispatch('user/setEcomStore', {
-          'eComStore': this.userProfile.stores.find((str: any) => str.productStoreId == store['detail'].value)
-        })
+        const productStore = this.userProfile.stores.find((str: any) => str.productStoreId === store['detail'].value);


We should not find the product store here, we could just pass the productStoreId here and action should handle if id is provided it finds the product store & set it and if product store is provided it should set it

Sure, sir, understood.

adityasharma7 · 2022-09-23T12:59:11Z

src/views/Settings.vue

      }
    },
    setShopifyConfig(event: any){
-      const shopifyConfig = this.shopifyConfigs.find((shopifyConfig: any) => shopifyConfig.shopifyConfigId === event.detail.value)
-      this.store.dispatch('user/setCurrentShopifyConfig', shopifyConfig);
+      const currentShopifyConfig = this.shopifyConfigs.find((shopifyConfig: any) => shopifyConfig.shopifyConfigId === event.detail.value);


Refer comments provided for the product store

Sure, sure.

adityasharma7 · 2022-09-23T13:00:03Z

src/store/modules/user/actions.ts

-  async setEcomStore({ commit, dispatch }, payload) {
+  async setEcomStore({ commit, dispatch, state }, productStore) {
+    const currentEComStore = state.currentEComStore;
+    const productStoreId = typeof productStore === 'string' ? productStore : productStore.productStoreId;


I would suggest we check if payload has productStoreId we find the productStore and if productStore we set it as is

Yes, sir, will change it as stated.

adityasharma7 · 2022-10-07T12:37:00Z

src/views/Settings.vue

        this.store.dispatch('user/setEcomStore', {
-          'eComStore': this.userProfile.stores.find((str: any) => str.productStoreId == store['detail'].value)
+          'productStoreId' : productStoreId ? productStoreId : {}


Why this is needed? We could just pass the productStoreId

adityasharma7 · 2022-10-07T12:37:31Z

src/views/Settings.vue

+      // const currentShopifyConfig = this.shopifyConfigs.find((shopifyConfig: any) => shopifyConfig.shopifyConfigId === event.detail.value);
+      const shopifyConfigId = event.detail.value;
+      this.store.dispatch('user/setCurrentShopifyConfig', {
+        'shopifyConfig' : shopifyConfigId ? shopifyConfigId : {}


Variable name should be shopifyConfigId
Why this is needed? We could just pass the shopifyConfigId

adityasharma7 · 2022-10-07T12:40:50Z

src/store/modules/user/actions.ts

+  async setCurrentShopifyConfig({ commit, dispatch, state }, payload) {
+    const shopifyConfigs = state.shopifyConfigs ? state.shopifyConfigs : {};
+    let currentShopifyConfig;
+    payload.shopifyConfig.shopifyConfigId ?


If we are getting shopifyConfig, just set it as is, if not find and set. Same applied to productStore

Made the changes, sir, please review.

adityasharma7 · 2022-10-10T07:00:37Z

src/store/modules/user/actions.ts

+    if(payload.productStore) {
+      commit(types.USER_CURRENT_ECOM_STORE_UPDATED, payload.productStore);
+    } else {
+      productStore = getters.getUserProfile.stores.find((store: any) => store.productStoreId === payload.productStoreId);


No need to use getter, I think we could directly access the state

adityasharma7 · 2022-10-10T07:01:54Z

src/store/modules/user/actions.ts

+    let productStore;
+
+    if(payload.productStore) {
+      commit(types.USER_CURRENT_ECOM_STORE_UPDATED, payload.productStore);


We should commit in both the cases, here we should assign productStore with payload.productStore

adityasharma7 · 2022-10-10T07:03:22Z

src/store/modules/user/actions.ts

+      productStore = getters.getUserProfile.stores.find((store: any) => store.productStoreId === payload.productStoreId);
+    }
+
+    dispatch('getShopifyConfig', productStore ? productStore.productStoreId : payload.productStore.productStoreId);


Once we have productStore set for both the cases, we will not need this ternary operation we could use productStore.productStoreId for both the cases

adityasharma7 · 2022-10-10T07:03:38Z

src/store/modules/user/actions.ts

    await UserService.setUserPreference({
      'userPrefTypeId': 'SELECTED_BRAND',
-      'userPrefValue': payload.eComStore.productStoreId
+      'userPrefValue': productStore ? productStore.productStoreId : payload.productStore.productStoreId


Same here. refer my previous comment

adityasharma7 · 2022-10-10T07:08:41Z

src/store/modules/user/actions.ts

+    if(payload.shopifyConfig) {
+      commit(types.USER_CURRENT_SHOPIFY_CONFIG_UPDATED, payload.shopifyConfig);
+    } else {
+      const shopifyConfigs = state.shopifyConfigs;
+      const currentShopifyConfig = shopifyConfigs.find((configs: any) => configs.shopifyConfigId === payload.shopifyConfigId)
+      commit(types.USER_CURRENT_SHOPIFY_CONFIG_UPDATED, currentShopifyConfig);
+    }


I would suggest following code for better readability

Suggested change

if(payload.shopifyConfig) {

commit(types.USER_CURRENT_SHOPIFY_CONFIG_UPDATED, payload.shopifyConfig);

} else {

const shopifyConfigs = state.shopifyConfigs;

const currentShopifyConfig = shopifyConfigs.find((configs: any) => configs.shopifyConfigId === payload.shopifyConfigId)

commit(types.USER_CURRENT_SHOPIFY_CONFIG_UPDATED, currentShopifyConfig);

}

let currentShopifyConfig = payload.shopifyConfig;

if(!currentShopifyConfig) {

currentShopifyConfig = state.shopifyConfigs.find((shopifyConfig: any) => shopifyConfig.shopifyConfigId === payload.shopifyConfigId)

}

commit(types.USER_CURRENT_SHOPIFY_CONFIG_UPDATED, currentShopifyConfig ? currentShopifyConfig : {});

adityasharma7 · 2022-10-10T07:10:50Z

src/views/Settings.vue

+        const productStoreId = store['detail'].value
+        this.store.dispatch('user/setEcomStore', { 'productStoreId': productStoreId })


Why not?

Suggested change

const productStoreId = store['detail'].value

this.store.dispatch('user/setEcomStore', { 'productStoreId': productStoreId })

this.store.dispatch('user/setEcomStore', { 'productStoreId': store['detail'].value })

Also, we could rename store to event like setShopifyConfig() method and access directly event.detail.value

adityasharma7 · 2022-10-10T07:12:10Z

src/views/Settings.vue

      }
    },
    setShopifyConfig(event: any){
-      const shopifyConfig = this.shopifyConfigs.find((shopifyConfig: any) => shopifyConfig.shopifyConfigId === event.detail.value)
-      this.store.dispatch('user/setCurrentShopifyConfig', shopifyConfig);
+      // const currentShopifyConfig = this.shopifyConfigs.find((shopifyConfig: any) => shopifyConfig.shopifyConfigId === event.detail.value);


Remove unwanted code

adityasharma7 · 2022-10-10T07:12:32Z

src/views/Settings.vue

+      const shopifyConfigId = event.detail.value;
+      this.store.dispatch('user/setCurrentShopifyConfig', { 'shopifyConfigId': shopifyConfigId });


Why not?

Suggested change

const shopifyConfigId = event.detail.value;

this.store.dispatch('user/setCurrentShopifyConfig', { 'shopifyConfigId': shopifyConfigId });

this.store.dispatch('user/setCurrentShopifyConfig', { 'shopifyConfigId': event.detail.value });

Implemented the changes, sir, please review.

…tters

src/store/modules/user/actions.ts

k2maan added 5 commits September 13, 2022 18:30

Implemented: Validation on runtime set by user to accept only future …

383b385

…date and time, instead of using minDateTime

Fixed: Indentations

25b04b9

Improved: Removed unnecessary code (method and getter) (#2tzb5w4)

06f37a2

Refactored: setEcomStore and setCurrentShopifyConfig actions to accep…

87dc4ed

…t ID as parameter (##2tzh44d)

Reverting: pushed unwanted file

0fd3d5c

adityasharma7 suggested changes Sep 23, 2022

View reviewed changes

Added find function with the desired usage

c7c082d

adityasharma7 suggested changes Oct 7, 2022

View reviewed changes

Refactored by removing and modifying unnecessary code

8d22484

adityasharma7 suggested changes Oct 10, 2022

View reviewed changes

k2maan added 2 commits October 10, 2022 16:43

Refactored code, improved variable names and used state instead of ge…

57fe77e

…tters

Refactored code and improved readability

e064808

adityasharma7 reviewed Oct 10, 2022

View reviewed changes

src/store/modules/user/actions.ts Outdated Show resolved Hide resolved

adityasharma7 added 2 commits October 10, 2022 18:13

Renamed: variable name for shopifyConfig as per the pattern (#2tzh44d))

a41a43a

Reverted: unwanted spaces (#2tzh44d)

e5385f1

adityasharma7 approved these changes Oct 10, 2022

View reviewed changes

adityasharma7 merged commit e9360e9 into hotwax:main Oct 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactored: setEcomStore and setShopifyConfig actions to accept ID. (#2tzh44d) #241

Refactored: setEcomStore and setShopifyConfig actions to accept ID. (#2tzh44d) #241

alsoK2maan commented Sep 20, 2022

adityasharma7 Sep 23, 2022

alsoK2maan Sep 24, 2022

adityasharma7 Sep 23, 2022

alsoK2maan Sep 24, 2022

adityasharma7 Sep 23, 2022

alsoK2maan Sep 24, 2022

adityasharma7 Oct 7, 2022

adityasharma7 Oct 7, 2022

adityasharma7 Oct 7, 2022

alsoK2maan Oct 7, 2022

adityasharma7 Oct 10, 2022

adityasharma7 Oct 10, 2022

adityasharma7 Oct 10, 2022

adityasharma7 Oct 10, 2022

adityasharma7 Oct 10, 2022

adityasharma7 Oct 10, 2022

adityasharma7 Oct 10, 2022

adityasharma7 Oct 10, 2022

adityasharma7 Oct 10, 2022

alsoK2maan Oct 10, 2022

		const productStoreId = store['detail'].value
		this.store.dispatch('user/setEcomStore', { 'productStoreId': productStoreId })

		const shopifyConfigId = event.detail.value;
		this.store.dispatch('user/setCurrentShopifyConfig', { 'shopifyConfigId': shopifyConfigId });

Refactored: setEcomStore and setShopifyConfig actions to accept ID. (#2tzh44d) #241

Refactored: setEcomStore and setShopifyConfig actions to accept ID. (#2tzh44d) #241

Conversation

alsoK2maan commented Sep 20, 2022

Related Issues

Short Description and Why It's Useful

Screenshots of Visual Changes before/after (If There Are Any)

Contribution and Currently Important Rules Acceptance

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment