Closed JDGrimes closed 7 years ago
I think that this is too much to tackle this late in a release. It would probably be better/easier after #264 and #404 anyway.
Now that #404 is in, this should be trivial to do.
(Actually, I'm not entirely sure that we don't end up deleting only the network-wide hooks, because maybe network mode is on when the points type is deleted.)
For the old points hooks, this was probably true. For the new points reactions, only the network hooks would be deleted as well, because in network context the standard reactions aren't allowed to be loaded.
I suppose that we could delete all orphaned hooks and reactions on update.
We were getting some strange PHPUnit failures, and I was ultimately able to trace them to this ticket.
3) WordPoints_Points_Type_Test::test_delete_deletes_reactions_multisite
Failed asserting that true is false.
/home/travis/build/WordPoints/wordpoints/tests/phpunit/tests/points/points-types.php:336
4) WordPoints_Points_1_4_0_Update_Test::test_custom_caps_added_when_network_active
Failed asserting that false is true.
/home/travis/build/WordPoints/wordpoints/tests/phpunit/tests/points/update/1-4-0.php:58
5) WordPoints_Points_1_4_0_Update_Test::test_standard_post_points_hooks_split_when_network_active
Failed asserting that Array &0 (
1 => Array &1 (
'points' => 20
'publish' => 20
'trash' => 25
'post_type' => 'ALL'
)
) is identical to Array &0 (
1 => Array &1 (
'points' => 20
'post_type' => 'ALL'
'auto_reverse' => 0
)
).
/home/travis/build/WordPoints/wordpoints/tests/phpunit/tests/points/update/1-4-0.php:152
6) WordPoints_Points_1_4_0_Update_Test::test_standard_comment_points_hooks_split_when_network_active
Failed asserting that Array &0 (
1 => Array &1 (
'points' => 10
'approve' => 20
'disapprove' => 25
)
) is identical to Array &0 (
1 => Array &1 (
'points' => 20
'post_type' => 'ALL'
'auto_reverse' => 0
)
).
/home/travis/build/WordPoints/wordpoints/tests/phpunit/tests/points/update/1-4-0.php:329
7) WordPoints_Points_1_5_0_Update_Test::test_custom_caps_added_to_new_sites
Failed asserting that false is true.
/home/travis/build/WordPoints/wordpoints/tests/phpunit/tests/points/update/1-5-0.php:50
I figured out that the issue was something relating to site switching, and apparently related to the switching done by our new routine classes, including the one that deletes a points type.
Further investigation finally revealed that it was related to the 'wordpoints_all_site_ids'
site transient used by get_all_site_ids()
being persisted between tests. In tests where we created a second site to be used in the tests, that site wasn't being swtiched to because only the ID of the main site (1
), was in the cached list.
It turns out that the cause of the persistence is the fact that we create some points types using our special fixture creation code that creates fixtures that can be used by all tests in a class. The fixtures are then deleted when the test case is finished, and when this happens the points type delete routine is run, causing the all site IDs cache to be filled in. Because this destruction of the fixtures takes place in tearDownAfterClass()
, any changes made there are persisted to all future tests, thus causing failures in future tests where other sites are added and are expected to be switched to by a routine.
This is slightly mitigated for the core update tests, because we actually delete the transient in the main testcase's update_wordpoints()
method. However, we don't in the component or extension update methods, so that's why some tests for the points component were failing. We also don't do anything for our new points type delete routine, which is why they were failing as well.
As it appears the get_sites()
is actually cached now, I think the best thing to do is just to remove the use of a transient of our own here entirely. So I'll open a separate ticket for that.
While working on https://github.com/WordPoints/hooks-api/issues/12, I've discovered that in
wordpoints_delete_points_type()
we don't correctly delete the points hooks when network active on multisite. When WordPoints is network active the points types are global across all sites in the network, but we only delete the points hooks on the current site. Also, we don't delete the network-wide hooks.(Actually, I'm not entirely sure that we don't end up deleting only the network-wide hooks, because maybe network mode is on when the points type is deleted.)
Related: #361