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

Finish detailing product form types along with commands #558

Open
wants to merge 2 commits into
base: 1.7.x
Choose a base branch
from

Conversation

jolelievre
Copy link
Contributor

Finish detailing product form types along with commands

| **PriceType** | `price` | Contains data about pricing | `UpdateProductPriceCommand` |
| **SpecificPriceType** | `add_specific_price` | Component to add a specific price | `AddProductSpecificPriceCommand` |
| **SpecificPricePriorityType** | `specific_price_priority` | Contains data about the priority order for specific prices | `UpdateSpecificPricePriorityCommand` |
| **StockType** | `stock` | Contains data about the product stock | `UpdateProductStockCommand` `UpdateProductWarehouseLocationCommand` (TBD should it be managed in stock form or separately) |
Copy link
Contributor

Choose a reason for hiding this comment

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

UpdateProductWarehouseLocationCommand there is still some logic related to warehouses ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to be still managed, I guess it is for backward compatibility because some Modules use it Just like advanced shop management It would be nice to have some sample modules to test this feature

@Progi1984 Progi1984 requested a review from matks June 9, 2020 15:24
| **BasicInformationType** | `basic` | Contains the basic product information of the Product | `AddProductCommand` and `UpdateProductBasicInformationCommand` |
| **DescriptionType** | `description` | Contains the description of the Product | `UpdateProductBasicInformationCommand` |
| **ShortcutType** | `shortcut` | Contains shortcut for prices, quantity and reference of the Product | `UpdateProductPriceCommand`, `UpdateProductStockCommand` and `UpdateProductOptionsCommand` |
| **TypeaheadProductPackCollectionType** | `pack_items` | List of products (for Pack of product) | `UpdateProductPackCommand` `CleanProductFromPackCommand` |
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no update action for a pack, we can only remove the packed product, so i would say:

Copy link
Contributor

@zuk3975 zuk3975 Jun 19, 2020

Choose a reason for hiding this comment

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

If we want some improvements, we could also add additional command like UpdatePackedProductQuantityCommand

Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note to close this discussion.
As discussed in slack, implemented UpdateProductPackCommand which resets the packs association completely to a new provided pack.

@matks matks changed the base branch from master to 1.7.x August 5, 2021 16:33
@matks
Copy link
Contributor

matks commented Aug 5, 2021

Hello,

I have modified your PR to target the 1.7.x branch.

We have modified the devdocs inner working in order to welcome multiple documentation versions. This is because soon we will have a 1.7 documentation and a 8.0 documentation, matching PrestaShop 1.7 or PrestaShop 8.0 softwares.

Read more on https://devdocs.prestashop.com/1.7/contribute/documentation/how/documentation-versions/

Please also note that now that we have multiple versions inside one documentation, we had to build a workflow that can handle this setup. This repository https://github.com/PrestaShop/docs now only contains the content of the documentation, and https://github.com/PrestaShop/devdocs-site contains the sources and the deployment workflow.

Because of this, some files have moved and you might have to rebase your Pull Request to match the new structure of the repository.

@kpodemski
Copy link
Contributor

Hi @jolelievre

Could you or someone else working on the product page take on this PR and finish it?

@kpodemski
Copy link
Contributor

Hi @jolelievre

Is this PR still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants