refactor: resolve all todos #1064
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- remove most obvious and outdated todos
bartoszherba
commented
May 26, 2022
| ApplyCouponToCartMutationVariables, | ||
| } from '../../types/GraphQL'; | ||
| import applyCouponToCartMutation from './applyCouponToCart'; | ||
| // @TODO : Add Mutation FROM CodeGEN |
Collaborator
Author
There was a problem hiding this comment.
I do not think that this is important
|
|
||
| const moduleOptions = JSON.parse('<%= JSON.stringify(options) %>'); | ||
|
|
||
| // TODO should be moved to THEME and expose consistent cookie management API |
Collaborator
Author
There was a problem hiding this comment.
By default, all todos in composable package are abandoned
Contributor
There was a problem hiding this comment.
this is already done anyway
| .addVirtualProductsToCart | ||
| .cart as unknown as Cart; | ||
| default: | ||
| // todo implement other options |
|
|
||
| if (externalCheckout.enable) { | ||
| if (userToken && cartToken) { | ||
| // @TODO: Implements Multiple Store |
Collaborator
Author
There was a problem hiding this comment.
We are going to work on the external checkout in dedicated tasks
|
|
||
| return bundleProductData?.addProductsToWishlist?.wishlist ?? {}; | ||
| default: | ||
| // todo implement other options |
| special: specialPrice || 0, | ||
| // @ts-ignore | ||
| credit: Math.round(specialPrice / 10), | ||
| // @TODO: Who set this installment value? |
Collaborator
Author
There was a problem hiding this comment.
By default, all todos in composable package are abandoned
| return null; | ||
| } | ||
|
|
||
| // TODO get correct data from api |
Collaborator
Author
There was a problem hiding this comment.
By default, all todos in composable package are abandoned
| () => state.value?.method_code && !isLoading.value, | ||
| ); | ||
| /** | ||
| * @TODO: Do not run the setShippingMethodsOnCart mutation on in-store pickup orders. |
Collaborator
Author
There was a problem hiding this comment.
I do not really understand this [05.2021]
| */ | ||
| query: Request; | ||
|
|
||
| // TODO: Add code sample |
Collaborator
Author
There was a problem hiding this comment.
We have the task for this M2-352
| const productPrice = computed(() => getGroupedProductPriceCommand(props.product)); | ||
| const productSpecialPrice = 0; // TODO add logic for special price calculation; | ||
| const productSpecialPrice = 0; |
Collaborator
Author
There was a problem hiding this comment.
This will be addressed M2-603
- remove AgnosticOrderStatus interface
- remove getFormattedPrice getter as it was not used
- remove ProductSearchParams interface
- remove loadWishlistItemsCounr params
- add new method in useWishlist - addOrRemoveItem
bartoszherba
force-pushed
the
M2-535-resolve-all-tod-os-in-code
branch
from
cc5f9bc to
ecdc77b
Compare
- update isInCart method to simplify expected parameters
- update customer login and registration types
bartoszherba
force-pushed
the
M2-535-resolve-all-tod-os-in-code
branch
from
9d2a658 to
db0d52a
Compare
- fix type issues in the shipping component
bartoszherba
force-pushed
the
M2-535-resolve-all-tod-os-in-code
branch
from
db0d52a to
44fc29a
Compare
bartoszherba
commented
May 26, 2022
| const numericAddressId = Number.parseInt(props.addressId, 10); // because in below find(), CustomerAddress['id'] is numeric | ||
| useFetch(async () => { | ||
| const addressesData = await addressesComposable.load(); // TODO don't fetch all addresses just to pluck one address |
Collaborator
Author
There was a problem hiding this comment.
It cannot be done, there is no way to get just one address.
bartoszherba
commented
May 26, 2022
| return; | ||
| } | ||
|
|
||
| if (!app.$vsf.$magento.config.state.getCustomerToken()) { // TODO: replace by value from pinia store after sueCart composable will be refactored |
Collaborator
Author
There was a problem hiding this comment.
I did, but then I got memory leaks and stack explosion, I left it as it is.
sethidden
previously approved these changes
May 26, 2022
Frodigo
previously approved these changes
May 27, 2022
bartoszherba
force-pushed
the
M2-535-resolve-all-tod-os-in-code
branch
from
67d9386 to
f79d67f
Compare
sethidden
approved these changes
May 27, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issue
Motivation and Context
Resolve TODOs
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: