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

Fix monitoring grids id configuration #15757

Merged
merged 2 commits into from Oct 15, 2019

Conversation

@zuk3975
Copy link
Contributor

zuk3975 commented Sep 30, 2019

Questions Answers
Branch? develop
Description? In grid_data_factory.yaml monitoring page grid definitions where configured with wrong id's. Also empty_category grid had one wrong argument injected, which was leading to error.
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? partially #10552
How to test? open /admin-dev/index.php/sell/catalog/monitoring, ensure that the page contains 7 lists. Each list should be filterable, sortable, paginatable independently from each other. The only action currently working is categories status toggling (other actions are not migrated).

This change is Reviewable

@zuk3975 zuk3975 requested a review from PrestaShop/prestashop-core-developers as a code owner Sep 30, 2019
@matks matks added this to the 1.7.7.0 milestone Sep 30, 2019
@matks matks referenced this pull request Oct 9, 2019
3 of 33 tasks complete
@Robin-Fischer-PS Robin-Fischer-PS self-assigned this Oct 10, 2019
@Robin-Fischer-PS

This comment has been minimized.

Copy link

Robin-Fischer-PS commented Oct 10, 2019

Hi @zuk3975 , thanks for the PR :)

I've noticed some problems :

  1. On all lists : When doing a partial search on the "ID" column (search "1" to find all ID containing 1), it doesn't work.

Here on Legacy, I have a result :
screenshot Monitoring with combi 1 - Legacy

On Symfony, no result :
screenshot Monitoring with combi 1 - Migration

  1. On "List of empty categories", there is a missing info on top of the list :
    "An empty category is a category that has no product directly associated to it. An empty category may however contain products through its subcategories."
    (see screenshot after 4.)

  2. On "List of empty categories", there is a sort missing, on "Description".
    (see screenshot after 4.)

  3. On "List of empty categories", in the description field, HTML tag appear in Symfony

Legacy :
screenshot Monitoring Category 1 - Legacy

Symfony :
screenshot Monitoring Category 1 -  Migration

Thanks :)

@zuk3975

This comment has been minimized.

Copy link
Contributor Author

zuk3975 commented Oct 11, 2019

@Robin-Fischer-PS nice catch 😄 . Filtering by id is not partial anymore(it searches for exact match) in all migrated lists, so it stays.
Ill check another bugs, thanks :)

… Add info alert on grid
@zuk3975 zuk3975 dismissed stale reviews from Ashek46, PierreRambaud, and matks via 127fce2 Oct 14, 2019
@zuk3975

This comment has been minimized.

Copy link
Contributor Author

zuk3975 commented Oct 14, 2019

@Robin-Fischer-PS, fixed.
One thing to notice though. The info block is placed as in legacy (inside grid card), but i've noticed for example in brands page, the info block is on top of the list (not in its card). Not sure which is the good way

@matks
matks approved these changes Oct 14, 2019
@Robin-Fischer-PS

This comment has been minimized.

Copy link

Robin-Fischer-PS commented Oct 15, 2019

Hi @zuk3975 ! Thanks for the fix, it's good for me :)

For the info block, since on this page it's only useful for the "Empty Categories", the actual design looks good and logical to me.

@PierreRambaud PierreRambaud merged commit b60f6ce into PrestaShop:develop Oct 15, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Oct 15, 2019

Thanks @zuk3975

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.