Page MenuHomeIn-Portal Phabricator
Differential D160

INP-1486 Don't give up, when single item update fails in "iterateItems" method
ClosedPublic
Actions

Authored by erik on Oct 16 2015, 4:23 AM.

Details

Reviewers
alex
Commits
rMINC16458: Bug INP-1486 - Don't give up, when single item update fails in "iterateItems"…
rINP16457: Fixes INP-1486 - Don't give up, when single item update fails in…
rMINL16459: Bug INP-1486 - Don't give up, when single item update fails in "iterateItems"…
Summary

removed code that set error status and prevents redirect

Test Plan

The D167 revision must be applied upfront for this test plan to work

Part 1

  1. go to Website & Content → Structure & Data section
  2. confirm that Up and Down buttons properly changes Order
  3. confirm that Approve and Decline buttons properly changes Status for categories and products
  4. go to Tools → Query Database section
  5. run SQL like: UPDATE inp_Categories SET l1_Name = '' WHERE l1_Name = 'Home' to break validation for some category in the middle of categories grid, displayed on Website & Content → Structure & Data section
  6. go to Website & Content → Structure & Data section
  7. confirm, that some category with status Active in the middle of the list have empty "Section Title" field
  8. select some categories with status Active, including category with empty "Section Title" field, some category before it and some category after it in the grid
  9. confirm that pressing Decline button do not change Status for category with empty name, and correctly change statuses for other selected categories, and no fatal error happens
  10. restore category name(s) and statuses, changed in this test

Part 2

  1. go to User Management → Users section
  2. create 4 users
  3. go to E-commerce → Affiliates section
  4. create an affiliate record for each of users, created above
  5. confirm that Approve and Decline buttons properly changes Status
  6. approve all affiliate records
  7. make sure there are 3+ affiliates in the grid, remember ID of some affiliate in the middle of the list
  8. go to Tools → Query Database section
  9. run SQL like: UPDATE inp_Affiliates SET SSN = '' WHERE AffiliateId = {remembered Id} to break validation for remembered affiliate in the middle of affiliates grid, displayed on E-commerce → Affiliates section
  10. go to E-commerce → Affiliates section
  11. open with Edit button remembered affiliate
  12. confirm, that SSN/Tax Id/VAT Number is empty
  13. go to E-commerce → Affiliates section
  14. select some affiliates including affiliate with empty SSN/Tax Id/VAT Number field, some affiliate before it and some affiliate after it in the grid
  15. confirm that pressing Decline button do not change Status for affiliate with empty SSN/Tax Id/VAT Number, and correctly change statuses for other selected affiliates, and no fatal error happens
  16. restore affiliate SSN/Tax Id/VAT Number and statuses, changed in this test

Part 3

  1. go to Website & Content → Structure & Data → Directory section
  2. click on Links tab
  3. add 4 links
  4. go to Directory → Link Validation section
  5. confirm that Approve and Decline buttons properly changes status icon in the Link Validation grid and Status field of related link
  6. approve all links
  7. make sure there are 3+ links in the grid, remember ID of some link in the middle of the list
  8. go to Tools → Query Database section
  9. run SQL like: UPDATE inp_Link SET Url = '' WHERE LinkId = {remembered Id} to break validation for remembered link in the middle of links grid, displayed on Directory → Link Validation section
  10. go to Directory → Link Validation section
  11. confirm, that Link URL is empty for remembered link
  12. select some links including link with empty Link URL field, some link before it and some link after it in the grid
  13. confirm that pressing Decline button do not change Status for link with empty Link URL, and correctly change statuses for other selected links, and no fatal error happens
  14. restore link Link URL and statuses, changed in this test

Part 4

  1. go to Website & Content → Structure & Data → Directory section
  2. click on Links tab
  3. confirm that Approve and Decline buttons properly changes status icon in the Links grid and Status field of related link
  4. approve all links
  5. make sure there are 3+ links in the grid, remember ID of some link in the middle of the list
  6. go to Tools → Query Database section
  7. run SQL like: UPDATE inp_Link SET Url = '' WHERE LinkId = {remembered Id} to break validation for remembered link in the middle of links grid, displayed on Website & Content → Structure & Data → Directory section
  8. go to Website & Content → Structure & Data → Directory section
  9. click on Links tab
  10. confirm, that Link URL is empty for remembered link
  11. select some links including link with empty Link URL field, some link before it and some link after it in the grid
  12. confirm that pressing Decline button do not change Status for link with empty Link URL, and correctly change statuses for other selected links, and no fatal error happens
  13. restore link Link URL and statuses, changed in this test

Part 5

  1. go to Directory → Paid Listings section
  2. add 4 listings using 4 links created before
  3. confirm that Approve and Decline buttons properly changes Status
  4. make all listings have status Active
  5. make sure there are 3+ listings in the grid, remember ID of some listing in the middle of the list
  6. go to Tools → Query Database section
  7. run SQL like: UPDATE inp_Listings SET ExpiresOn = NULL WHERE ListingId = {remembered Id} to break validation for remembered listing in the middle of listings grid, displayed on Directory → Paid Listings section
  8. go to Directory → Paid Listings section
  9. confirm, that Expires On is empty for remembered listing
  10. select some listings including listing with empty Expires On field, some listing before it and some listing after it in the grid
  11. confirm that pressing Decline button do not change Status for listing with empty Expires On, and correctly change statuses for other selected listings, and no fatal error happens
  12. restore listing Expires On and statuses, changed in this test

Diff Detail

Repository
rINP In-Portal
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

erik updated this revision to Diff 376.Oct 16 2015, 4:23 AM
erik retitled this revision from to INP-1486 Don't give up, when single item update fails in "iterateItems" method.
erik updated this object.
erik edited the test plan for this revision. (Show Details)
erik added a reviewer: alex.
erik added 1 JIRA issue(s): INP-1486.
alex requested changes to this revision.Nov 5 2015, 2:36 AM
alex edited edge metadata.
  1. test plan is missing tests, that demonstrate, that problem, described in Confluence, no longer happen under same conditions
  2. please remove ShippingQuoteEngineEventHandler::iterateItems method because the only thing it does it rolling back event status change, that no longer happens in parent class method
This revision now requires changes to proceed.Nov 5 2015, 2:36 AM
erik edited the test plan for this revision. (Show Details)Nov 9 2015, 4:53 AM
erik edited edge metadata.
erik updated this revision to Diff 405.Nov 9 2015, 5:04 AM
erik edited edge metadata.

Removed excessive code from shipping_quote_engine_event_handler.php

alex requested changes to this revision.EditedNov 10 2015, 2:27 AM
alex edited edge metadata.
  1. add temporary line "$event->status = kEvent::erFAIL;" to CategoriesEventHandler::OnBeforeItemUpdate
  2. confirm that Approve and Decline buttons do not change Status for categories, but also no fatal error happens
  3. remove temporary line "$event->status = kEvent::erFAIL;" from CategoriesEventHandler::OnBeforeItemUpdate

Following addition to the test plan does test something, but not the originally failing scenario, in which:

  • several items were selected in grid (e.g. 5)
  • only item in the middle (e.g. 3rd) had validation error
  • only items before one with validation error were updated in db

After the fix all items (except one with validation error) must be updated in DB and test plan needs to cover that scenario.

This revision now requires changes to proceed.Nov 10 2015, 2:27 AM
erik edited the test plan for this revision. (Show Details)Nov 10 2015, 4:34 AM
erik edited edge metadata.
erik updated this revision to Diff 408.Nov 10 2015, 4:39 AM
erik edited edge metadata.
erik edited the test plan for this revision. (Show Details)

Changed test plan, added Clear() calls to fit new plan.

alex requested changes to this revision.Nov 16 2015, 3:04 AM
alex edited edge metadata.

... added Clear() calls to fit new plan.

Thanks for discovering, that without Clear() call before Load() call the validation errors are not reset and blocks upcoming updates. This unfortunately is out of the scope of this task and therefore:

  • I've created a discussion about it specifically http://community.in-portal.org/x/2gH3 and a task in JIRA that I've assiged to you
  • please do the following:
    1. undo latest changes (where Clear() method call was added)
    2. create differential revision for http://jira.in-portal.org/browse/INP-1514 task
    3. update test plan to say, that above created revision must be applied upfront for this test plan to work

Changed test plan

Test plan does include the needed test case with failed validation, but unfortunately this happens only for 1 of 4 changes places. It would be best to create mini test plans (for each section using sub-heading markup, see below) where complete test scenario (including relevant SQLs to emulate validation error) for following cases would be written:

  • no validation error and selected records were updated
  • validation error in non-last record and all but invalid records were updated

The sub-heading markup looks like this:

## sub-heading one

# plan item 1
# plan item 2


## sub-heading two

# plan item 1
# plan item 2
This revision now requires changes to proceed.Nov 16 2015, 3:04 AM
erik updated this revision to Diff 412.Nov 16 2015, 4:20 AM
erik edited edge metadata.

Removed Clear() calls, added in the previous version of revision.

alex requested changes to this revision.Nov 17 2015, 3:03 AM
alex edited edge metadata.

The test plan wasn't changed as requested.

This revision now requires changes to proceed.Nov 17 2015, 3:03 AM
alex added inline comments.Nov 17 2015, 3:10 AM
modules/in-link/units/listings/listings_event_handler.php
612 ↗(On Diff #412)

Please remove this line.

Same changes was done in other, but not this place.

erik edited the test plan for this revision. (Show Details)Nov 17 2015, 4:21 AM
erik edited edge metadata.
erik updated this revision to Diff 417.Nov 17 2015, 4:29 AM
erik edited edge metadata.
erik edited the test plan for this revision. (Show Details)

Removed excessive line, improved test plan before this revision

alex edited the test plan for this revision. (Show Details)Nov 17 2015, 4:49 AM
alex edited edge metadata.
alex added a project: Restricted Project.Mar 10 2016, 5:20 AM
alex edited the test plan for this revision. (Show Details)Nov 19 2016, 1:27 PM
alex edited the test plan for this revision. (Show Details)Nov 20 2016, 3:37 AM
alex edited the test plan for this revision. (Show Details)Nov 20 2016, 3:47 AM
alex edited the test plan for this revision. (Show Details)Nov 20 2016, 3:52 AM
alex edited the test plan for this revision. (Show Details)Nov 20 2016, 4:45 AM
alex edited the test plan for this revision. (Show Details)Nov 20 2016, 4:55 AM
alex edited the test plan for this revision. (Show Details)Nov 20 2016, 5:15 AM
alex accepted this revision.Nov 20 2016, 5:18 AM
alex edited edge metadata.
This revision is now accepted and ready to land.Nov 20 2016, 5:18 AM
Closed by commit rINP16457: Fixes INP-1486 - Don't give up, when single item update fails in… (authored by alex). · Explain WhyNov 24 2016, 11:05 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

DiffIDBaseDescriptionCreatedLintUnit
BaseBase
Diff 137616285Oct 16 2015, 4:23 AM
★
★
Diff 240516300Removed excessive code from shipping_quote_engine_event_handler.phpNov 9 2015, 5:04 AM
★
★
Diff 340816300Changed test plan, added Clear() calls to fit new plan.Nov 10 2015, 4:39 AM
★
★
Diff 441216300Removed Clear() calls, added in the previous version of revision.Nov 16 2015, 4:20 AM
★
★
Diff 541716300Removed excessive line, improved test plan before this revisionNov 17 2015, 4:29 AM
★
★
Diff 662516456rINP16457Nov 24 2016, 11:05 AM
★
★

Diff 625

View Options

branches/5.2.x/core/kernel/db/cat_event_handler.php

Loading...
View Options

branches/5.2.x/core/kernel/db/db_event_handler.php

Loading...
View Options

branches/5.2.x/core/units/categories/categories_event_handler.php

Loading...
In-Portal CMS · In-Portal Community Pages · In-Portal Issue Tracker · In-Portal Repository Browser · Quality Assurance · In-Portal CI Server