Attachment iframe modal

[jolelievre]

Questions	Answers
Branch?	develop
Description?	This PR is based on this initial one #24776 but is a POC to test if it's doable to handle the whole form inside the iframe and still get the ID of the created entity from the parent, then close the modal This is done by creating a new component IframeModal which uses an URL as initial parameter and displays an Iframe inside a modal, the it will trigger a callback on each new page load (a spinner is displayed while loading)
Type?	improvement
Category?	BO
BC breaks?	yes
Deprecations?	yes
Fixed ticket?	~
How to test?	Go to tab Specifications, you can edit the attachments either by searching existing ones or by adding new ones via a modal (note: after edition you must save the product form to validate your modifications)
Possible impacts?	It changes the modal functions into TypeScript classes, without changing the initial use of ConfirmModal so it shouldn't break anything but it'd be safer to check a few different modals to be sure nothing has been broken

---

This change is 

Breaking Changes

• PrestaShop\PrestaShop\Adapter\Attachment\AttachmentRepository now has two parameters in its constructor
• AttachmentInformation query result objects now expects (and contains) localized values for names, it also contains localized description and size And getType was renamed getMimeType
• PrestaShop\PrestaShop\Core\Domain\Product\QueryResult\ProductForEditing now expects (and contains) an array of AttachmentInformation instead of an array of attachment IDs
• Removed useless AssociateProductAttachmentCommand command and GetAttachmentInformationList query
• Move RemoveAllAssociatedProductAttachmentsCommand and SetAssociatedProductAttachmentsCommand into dedicated Attachment sub namespace

Known issue

The form iframe component has a small integration bug, a dedicated issue was created for this #25735

[prestonBot]

Hi, thanks for this contribution!

I found some issues with the Pull Request description:

• Your pull request does not seem to fix any issue, consider creating one (see note below) and linking it by writing Fixes #1234.

Would you mind having a look at it? This will help us understand how interesting your contribution is, thank you very much!

About linked issues

Please consider opening an issue before submitting a Pull Request:

• If it's a bug fix, it helps maintainers verify that the bug is effectively due to a defect in the code, and that it hasn't been fixed already.
• It can help trigger a discussion about the best implementation path before a single line of code is written.
• It may lead the Core Product team to mark that issue as a priority, further attracting the maintainers' attention.

(Note: this is an automated message, but answering it will reach a real human)

[prestonBot]

This pull request seems to contain new translation strings. I have summarized them below to ease up review:

• Admin.Catalog.Feature
  • Search file
  • Attachment was successfully created and added to the selection.
  • Select the files (instructions, documentation, recipes, etc.) your customers can directly download on this product page.
  • Need to browse all files? Go to [1]Catalog > Files[/1]
• Admin.Global
  • This window will close automatically.

(Note: this is an automated message, but answering it will reach a real human)

[jolelievre]

The POC is not over yet but here is a preview of what it looks like at the moment this comment is written:

https://drive.google.com/file/d/1Ph2T8Zqw7QQYFYgBbGOz9_Zpg_xpLQt9/view

I'd like first impressions of this? Though I already have a few improvements ideas:

• The spinner layer might be fully blank, the current transparency, although nice while waiting, is maybe confusing when the successful result is displayed behind

• I was thinking of creating a new FormIframeModal that would extend the new IframeModal and would handle internally the check of the form, it would then provide a callback on load with onForm(form: HTMLElement, formData: SerializedData, formDataAttributes: Dataset): void

• This component could also be able to detect the success/error alert boxes, and would provide some helper functions to drive the modal behaviour, thus the parent could display only this alert message before closing the modal automatically:

  • hasAlert() hasSuccessAlert() hasErrorAlert()
  • displaySuccessAlert displayErrorAlert

• Maybe all this is overkill and a more simple thing would be to provide a simple function that allows to close the IframeModal after a message is displayed in place of the iframe like displayMessageAndClose("The attachment was successfully created and added to the selection list. The window will now close automatically")

(Edit)

• Another feature that would be useful if we have a FormIframeModal watch the click on cancel to close the modal, if we don't have a dedicated class for form, then it's the parent which should check for this click after each load

[zuk3975]

So after its done, it quickly redirects to edit page and collects the info, then closes it.
I just don't like that this part is visible. It seems like "oops you shouldn't have seen this".
Though this way we have errors displayed properly (and I suppose original "createAction" didn't need to be modified to support ajax. So if its fine about that "visible redirection", then it seems to be better approach than having ajax stuff as in my pr

[Julievrz]

Wording ✔️, thanks!

[jolelievre]

Okay, I finished the FormIframeModal component, it matches exactly what I had in mind Here is a preview of the result:
https://drive.google.com/file/d/1koIi2iTC1sFVyFf0DtVJsRapS5-HKw4V/view

The component was designed to be easy to use, basically:

• set the form URL
• one JS callback on form loaded with the form, its data attributes and serialized form data to meet multiple use case
• some helper functions to easily hide/show the modal
• a helper function to easily display a message (useful to display success message before closing the modal)

@MatShir @zuk3975 let me know what you think of it?

[NeOMakinG]

Only 2 feedbacks for me:

• Let's keep consistent and type void any void methods/functions, even if it's not asked by TS
• Maybe this require an ADR, but I would love to use a variable in order to name CSS component and even twig components, for this one it's not that important since it's a bootstrap one, but for custom ones we could use BEM-like philosophy as bootstrap does and use ${component-name} prefixed classes like I did there for the FO Add availability component to FO #25062

[khouloudbelguith]

@jolelievre, I just updated my comment to reproduce the first issue.

Thanks!

[khouloudbelguith]

Hi @MatShir,

We need your confirmation about this issue

Create some files in the BO > Catalog > Files page
Enable Experimental product page
Go to BO > Catalog > Products
Click on Add New product on the experimental page
Set the tile without saving
In the Specifications tab
Try to search any file => NOK (files are not displayed)
Try to add a new file => NOK: the page is displayed on another page

https://drive.google.com/file/d/1lma0pzrT4eATD5KYxEsnD0PYZr4CD_Mm/view?usp=sharing

Is it ok to accept or do we need to fix the issue?

Thanks!

[MatShir]

Hi @MatShir,

We need your confirmation about this issue

Create some files in the BO > Catalog > Files page
Enable Experimental product page
Go to BO > Catalog > Products
Click on Add New product on the experimental page
Set the tile without saving
In the Specifications tab
Try to search any file => NOK (files are not displayed)
Try to add a new file => NOK: the page is displayed on another page

https://drive.google.com/file/d/1lma0pzrT4eATD5KYxEsnD0PYZr4CD_Mm/view?usp=sharing

Is it ok to accept or do we need to fix the issue?

Thanks!

This behavior is not supposed to happen. The creation won't part of the BO anymore. The creation page would be replaced by a popup. So If the behavior works fine while editing the product page, then I think this PR is ready :)

[khouloudbelguith]

Hi @jolelievre,

After these modifications, the exception is not displayed anymore.
But we can select the file multiple time.

Steps to reproduce the issue:

1. Create some files in the BO > Catalog > Files page
2. Enable Experimental product page
3. Go to BO > Catalog > Products
4. Edit a product on the experimental page
5. In the Specifications tab > search a file and click on it
6. search the same file and click on it again
7. search the same file and click on it again
8. search the same file and click on it again

=> can be selected multiple times: NOK

If we click save => it will be ok

Ping @MatShir we need your confirmation, is it a bug? can we accept this issue?
As discussed with @jolelievre we have a related issue (similar behavior in this ticket #9617 (comment)

Thanks!

[jolelievre]

@MatShir to complement Khouloud's comment indeed a file can be selected multiple times, but on save only one is associated without exception and so on refresh only one is visible

But it's not an ideal behaviour so to avoid blocking this PR I suggest adding this bug into this issue #25735 which already contains some bug that needs to be fixed on the attachment modal Or we can open a dedicated issue

I'd like to merge this PR quickly because it contains a few components that are needed for categories, and it also blocks the Pack and related products feature (so the bug could also be fixed along with related products)

[MatShir]

Hi @jolelievre,

After these modifications, the exception is not displayed anymore.
But we can select the file multiple time.

Steps to reproduce the issue:

1. Create some files in the BO > Catalog > Files page
2. Enable Experimental product page
3. Go to BO > Catalog > Products
4. Edit a product on the experimental page
5. In the Specifications tab > search a file and click on it
6. search the same file and click on it again
7. search the same file and click on it again
8. search the same file and click on it again

=> can be selected multiple times: NOK

If we click save => it will be ok

Ping @MatShir we need your confirmation, is it a bug? can we accept this issue?
As discussed with @jolelievre we have a related issue (similar behavior in this ticket #9617 (comment)

Thanks!

@khouloudbelguith Could you make a new issue ? If it is the only issue remaining, we are going to fix it later. This PR need to go forward. Thanks 🤗

[jolelievre]

I created the appropriate issue here #25831
To be fixed in another PR

[khouloudbelguith]

Hi @jolelievre,

Yes, it is ok ✔️

1. I checked the search => ok only this issue: Product page v2: handle duplicate attachments #25831
2. Edit a product + Add a new frame:

• File size > Max upload size => ok
• Empty name => ok
• Empty field => ok
• invalid name => ok
• valid file + valid name => ok

=> the file is well added in the BO > Catalog > files page
I save the product => in the FO, file well attached

3. When I remove a file from a product and save => ok

4. I checked other modal forms:

• Modal confirmation to Delete image
• Modal confirmation to delete customization
• Modal confirmation to delete a product

In the BO > Orders > Add new Order page:

• Modal to add new customer
• Modal to add new address
• Modal o show Order detail

Thanks!

[prestashop-issue-bot]

QA OK without required approvals !?

[prestashop-issue-bot]

QA OK without required approvals !?