-
Notifications
You must be signed in to change notification settings - Fork 18
Integrated basic info with API #236
base: master
Are you sure you want to change the base?
Conversation
defudef
commented
Mar 6, 2020
•
edited
Loading
edited
- I can see personal data on my account page
- I can see all my addresses from CT
- I can see which address is masked for shipment and which one is for billing
- I want to see my email address on password change subpage
- Refactor MyAccount.vue (using setup instead of data and methods)
- Refactor ShippingDetails.vue (using setup instead of data and methods)
- Unit tests
related #224 |
packages/core/theme-module/theme/pages/MyAccount/ShippingDetails.vue
Outdated
Show resolved
Hide resolved
…my-account-api-integration
packages/core/theme-module/theme/pages/MyAccount/ShippingDetails.vue
Outdated
Show resolved
Hide resolved
…my-account-api-integration
…my-account-api-integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok, but I think it may require an RFC
packages/commercetools/composables/tests/useUserAddress/useUserAddress.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes from packages/core/
need to be proposed first as RFC and then after discussion can be further implemented
import {UseUserFactoryParams} from '@vue-storefront/factories'; | ||
import {Customer} from '@vue-storefront/commercetools-api/lib//types/GraphQL'; | ||
import { UseUserFactoryParams } from '@vue-storefront/factories'; | ||
import { Customer } from '@vue-storefront/commercetools-api/lib//types/GraphQL'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import { Customer } from '@vue-storefront/commercetools-api/lib//types/GraphQL'; | |
import { Customer } from '@vue-storefront/commercetools-api/lib/types/GraphQL'; |
@@ -10,7 +11,9 @@ import { | |||
} from '@vue-storefront/commercetools-api'; | |||
import useCart from '../useCart'; | |||
|
|||
export const params: UseUserFactoryParams<Customer, any, any> = { | |||
export type UpdateUserParams = Customer & { clientSideOnly?: boolean }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you elaborate more on that?
|
||
export const params: UseUserAddressFactoryParams<Customer, Address, UserAddressType, { [x: string]: any }, UpdateUserParams> = { | ||
userComposable, | ||
addresses: userComposable.user.value && userComposable.user.value.id ? userComposable.user.value.addresses : [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user
is a computed, readonly property, you can't assign anything to it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will need to partailly implement #342 for useUser
const loading: Ref<boolean> = ref(false); | ||
|
||
const updateData = async (data: UpdatedUserAddresses<ADDRESS, UPDATE_USER_PARAMS>) => { | ||
factoryParams.addresses.length = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SEARCH_PARAMS = { [x: string]: any }, | ||
UPDATE_USER_PARAMS = any, | ||
> = { | ||
addresses: ADDRESS[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why you need to explicitly pass those addresses? Shouldn't that be loaded by searchAddresses
?
addAddress, | ||
updateAddress, | ||
deleteAddress, | ||
getBillingAddresses, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and getShippingAddresses
is not a part of a public interface, it could be a getter or computed property or a field within addresses
(to be discussed via RFC) but definitely not a public function. It's also not following architectural approach for compostable https://vsf-next-docs.netlify.com/commercetools/composables.html
root.$router.push(`/my-account/${title.toLowerCase().replace(' ', '-')}`); | ||
}; | ||
|
||
const account = computed(() => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you can't just pass account
and avoid additional code while its not doing anything meaningful?
? account.getShippingAddresses().find(shippingAddress => id === shippingAddress.id) | ||
: {}; | ||
|
||
formType.value = id === null ? 'CREATE' : 'UPDATE'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you explain why you need that?
@@ -1,18 +1,18 @@ | |||
<template> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally this file is not easily readable, please think about cleaning it up
@@ -0,0 +1,97 @@ | |||
import { UseUserAddress, UserAddressType, UseUser } from '@vue-storefront/interfaces'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please work on readability of this code, it's really hard to get some things