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

Display NOK PrestaShop requirements #9407

Merged
merged 6 commits into from Aug 22, 2018

Conversation

Projects
None yet
8 participants
@Quetzacoalt91
Member

Quetzacoalt91 commented Aug 6, 2018

Questions Answers
Branch? develop
Description? Display at least the check "name" when a PrestaShop prerequisite is non-ok, event if there is no description text. This PR can be completed by covering all checks, its main purpose is to always display an information to the user, even incomplete.
Type? bug fix
Category? BO
BC breaks? Nope
Deprecations? Nope
Fixed ticket? http://build.prestashop.com/news/coreweekly-week-30-2018/#comment-4017968715
How to test? Disable the fileinfo extension and reach the information page in your back office. Without the PR, the list is empty. With it, you know at least what was the test made.

capture du 2018-08-06 17-36-59


This change is Reviewable

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 6, 2018

Contributor

While I agree it's better to display an incomplete information than nothing, I'd like to see this hack outside of Twig template.

Can't this be done before in the application workflow?

Contributor

mickaelandrieu commented Aug 6, 2018

While I agree it's better to display an incomplete information than nothing, I'd like to see this hack outside of Twig template.

Can't this be done before in the application workflow?

@marionf marionf assigned marionf and unassigned marionf Aug 16, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Aug 16, 2018

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Aug 17, 2018

Member

One more review from @LouiseBonnard and we're good. I saw 2 domains were wrong.

Member

Quetzacoalt91 commented Aug 17, 2018

One more review from @LouiseBonnard and we're good. I saw 2 domains were wrong.

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 21, 2018

Contributor

so be it, I can't merge without @LouiseBonnard approval ^^

Contributor

mickaelandrieu commented Aug 21, 2018

so be it, I can't merge without @LouiseBonnard approval ^^

@PierreRambaud PierreRambaud merged commit 50e5e9f into PrestaShop:develop Aug 22, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@PierreRambaud

This comment has been minimized.

Show comment
Hide comment
@PierreRambaud

PierreRambaud Aug 22, 2018

Contributor

Thanks @Quetzacoalt91 and everyone for reviews :)

Contributor

PierreRambaud commented Aug 22, 2018

Thanks @Quetzacoalt91 and everyone for reviews :)

@Prestapro

This comment has been minimized.

Show comment
Hide comment
@Prestapro

Prestapro Aug 28, 2018

Hello,

I have the same error on 2 different stores, do you have a solution?

Prestapro commented Aug 28, 2018

Hello,

I have the same error on 2 different stores, do you have a solution?

@Quetzacoalt91 Quetzacoalt91 deleted the Quetzacoalt91:display-system-requirement-nok branch Aug 28, 2018

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Aug 28, 2018

Member

You can temporarily apply the changes suggested in bdd7bb4.

The actual error will appear on the page.

Member

Quetzacoalt91 commented Aug 28, 2018

You can temporarily apply the changes suggested in bdd7bb4.

The actual error will appear on the page.

@rdy4ever

This comment has been minimized.

Show comment
Hide comment
@rdy4ever

rdy4ever Sep 8, 2018

Contributor

Works great! Thanks

Contributor

rdy4ever commented Sep 8, 2018

Works great! Thanks

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