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

Implemented functionality for find order page ( #1wn444v ) #4

Closed
wants to merge 20 commits into from
Closed

Implemented functionality for find order page ( #1wn444v ) #4

wants to merge 20 commits into from

Conversation

meet-aniket
Copy link
Contributor

@meet-aniket meet-aniket commented Dec 15, 2021

Implemented functionality in find order page, I have used findOrder and searchProduct endpoint there and integrated all necessary functionalities but did not get some properties there.

changelogs/unreleased/-1wn444v.yml Outdated Show resolved Hide resolved
@@ -0,0 +1,115 @@
<template>
<div class="order-header">
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Improved Sir

IonNote,
OpenItemCard,
},
props: ['order'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Improved sir

// name: 'ProductInventory',
// component: ProductInventory,
// beforeEnter: authGuard
// },
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this if not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed Sir

IonThumbnail,
IonTitle,
IonToolbar,
IonSelectOption,
Copy link
Contributor

Choose a reason for hiding this comment

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

Arrange the component import alphabetically

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arranged Sir

src/main.ts Outdated
@@ -57,7 +57,7 @@ app.config.globalProperties.$filters = {
externalId = externalIdentificationSplit[1] ? externalIdentificationSplit[1] : '';
}
return externalId;
},
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this unwanted change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed Sir.

@@ -40,12 +40,48 @@ const actions: ActionTree<ProductState, RootState> = {
// Remove added loader only when new query and not the infinite scroll
if (payload.viewIndex === 0) emitter.emit("dismissLoader");
} catch(error){
console.log(error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we have removed this console statement?

Copy link
Contributor Author

@meet-aniket meet-aniket Dec 21, 2021

Choose a reason for hiding this comment

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

Aditya sharma Sir instructed me to remove all console.log from code before making PR.

Copy link
Contributor

@ymaheshwari1 ymaheshwari1 Dec 21, 2021

Choose a reason for hiding this comment

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

But this console statement is not added by you and it's used to display the error message

products: {
list: any;
total: number;
}
productsInformation: any;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this state is not needed.

Copy link
Contributor Author

@meet-aniket meet-aniket Dec 21, 2021

Choose a reason for hiding this comment

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

Sir according to me this statement is removed in recent commits please check those


// Add Product information in cache

async getProductInformation(context, { orders }){
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove duplicate declaration of the same function. You have defined the getProductInformation here in the order action and also in the product action.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay sir will update

products: {
list: {},
total: 0
}
},
productsInformation: {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the productsInformation state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already removed Sir

},
[types.PRODUCT_ADD_TO_CACHED_MULTIPLE] (state, payload) {
// TODO
if (payload.products) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Improve indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Improved sir

}
},
[types.PRODUCT_ADD_TO_CACHED_MULTIPLE] (state, payload) {
// TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

Why here a TODO statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed Sir

},

methods: {
async getOrders(vSize, vIndex){
Copy link
Contributor

Choose a reason for hiding this comment

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

Make vSize and vIndex optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okey sir

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sir made them optional


},
mounted() {
this.getOrders(10, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

We won't pass the view size and view index from the mounted hook

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, Sir

…order and made getOrder function's parameters optional (#1wn444v)
@meet-aniket
Copy link
Contributor Author

Screenshot from 2021-12-21 18-18-31
Screenshot from 2021-12-21 18-17-32

Copy link
Contributor

@ymaheshwari1 ymaheshwari1 left a comment

Choose a reason for hiding this comment

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

Remove package-lock.json file changes.

@meet-aniket meet-aniket closed this Jan 5, 2022
@meet-aniket
Copy link
Contributor Author

This pull request can not be accessed by me for making further changes so this is why I am closing this pull request.

ymaheshwari1 pushed a commit to ymaheshwari1/ionic-commerce-hub that referenced this pull request Feb 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants