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

[NEW UI] add middleware system and first middleware on update options page #1136

Merged
merged 11 commits into from
Jan 29, 2025

Conversation

ga-devfront
Copy link
Contributor

@ga-devfront ga-devfront commented Jan 24, 2025

Questions Answers
Description? Add middleware system and the first middleware on update options page to test if configuration exist and if it's local version if xml and zip exists.
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? N/A
Sponsor company @PrestaShopCorp
How to test? see bellow

Test Steps for Update Options Page Redirection Logic

  1. Navigate to the module homepage.

  2. Test without a version choice:

    • Go to the update version choice page without picking any version.
    • Manually change the route parameter in the URL to update-page-update-options (replacing update-page-version-choice).
    • Expected result: You are redirected back to update-page-version-choice.
  3. Test with the online channel:

    • Select the online channel.
    • Change the route parameter in the URL to update-page-update-options.
    • Expected result: You can access update-page-update-options.
  4. Test with the local channel:

    • Return to update-page-version-choice.
    • Select the local channel.
    • Change the route parameter in the URL to update-page-update-options.
    • Expected result: You can access update-page-update-options.
  5. Test invalid file references:

    • Return to update-page-version-choice.
    • Without making any changes to the channel selection, delete the XML or ZIP file from your server that was previously picked.
    • Change the route parameter in the URL to update-page-update-options.
    • Expected result: You are redirected back to update-page-version-choice.

PS: If you have already a configuration set you can delete it manually from you server by removing /admin-folder/autoupgrade/update_config.varl

@ga-devfront ga-devfront added this to the 7.0.0 milestone Jan 24, 2025
@ga-devfront ga-devfront changed the title [NEW UI] addd middlware [NEW UI] add middleware system and first middleware on update options page Jan 24, 2025
classes/Router/RoutesConfig.php Outdated Show resolved Hide resolved
classes/Router/RoutesMiddleware.php Outdated Show resolved Hide resolved
classes/Router/Router.php Outdated Show resolved Hide resolved
classes/Router/Router.php Outdated Show resolved Hide resolved
classes/Router/Router.php Outdated Show resolved Hide resolved
classes/Router/Router.php Outdated Show resolved Hide resolved
classes/Router/Router.php Outdated Show resolved Hide resolved

class UpdateIsConfigured extends AbstractMiddleware
{
public function process(): ?string
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public function process(): ?string
/**
* @return Routes::*|null
*/
public function process(): ?string


use PrestaShop\Module\AutoUpgrade\Router\Routes;

class LocalChannelXmlAndZipExist extends AbstractMiddleware
Copy link
Contributor

Choose a reason for hiding this comment

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

The validation function does more than just check the presence of the zip/xml pair, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and no, it check validty only if we are on channel local.

Copy link
Contributor

Choose a reason for hiding this comment

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

for me in addition to validating the presence of files, it also validates the presence of versions and the match of this one. So the middleware doesn't really reflect what is being done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You think LocalChannelXmlAndZipAreValid is better ?

classes/Router/Router.php Show resolved Hide resolved
classes/Router/UrlGenerator.php Show resolved Hide resolved
classes/Router/UrlGenerator.php Show resolved Hide resolved
Copy link
Contributor

@AureRita AureRita left a comment

Choose a reason for hiding this comment

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

Hi @ga-devfront

Thank you for your PR, I tested it and it seems to works as you can see :

recording.52.webm

I also tested when you delete the file "update_config.var", and it works as expected ( You have to select again your version destination )

Tested from :
8.0.4 to 8.2
8.2 to 9.0.0
8.0.4 to 9.0.0

Because the PR seems to works as expected, It's QA ✔️

Thank you

@ga-devfront ga-devfront merged commit acf0bb2 into PrestaShop:dev Jan 29, 2025
37 checks passed
@ga-devfront ga-devfront deleted the feat/add-middlware branch January 29, 2025 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready for review
Development

Successfully merging this pull request may close these issues.

5 participants