Server::removeIndex() won't be called since the server is already marked as disabled internally.Server::removeIndex() won't be called.Server::removeIndex() ever having been called, possibly resulting in stale data or left-over tables.Possible solution: call removeIndex() regardless of server status and move the status check there, moving the call to the pending server tasks if the server is currently disabled.
Alternately, we could there also check for $this->original->status() to see if we are currently in the process of disabling and, if we are, forward the call regardless. Might be a bit risky, though, depending on why the server is being disabled. But, since we have the same problem when deleting (even worse there, I guess, so at the time of disabling is probably the safer bet), it's also an idea.
In general, it seems the server doesn't check its own status at all, relying on the index to regard those restrictions (without them being documented anywhere, as far as I can see). Definitely also something to look into.
This issue was identified as a Beta Blocker for Drupal 8. We sat down and figured out the value proposition and amount of work (story points) for this issue.
Value and Story points are in the scale of fibonacci. Our minimum is 1, our maximum is 21. The higher, the more value or work a certain issue has.
Value : 2
Story Points: 3
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 2318169-3--server_add_remove_index_tests.patch | 4.95 KB | drunken monkey |
Comments
Comment #1
drunken monkeyComment #2
nick_vhComment #3
drunken monkeySeems like this has already been fixed at some point. But adding tests to verify this still seems like a good idea.
Comment #4
borisson_@inheritdoc
that's all I'd like to see changed.
Comment #6
drunken monkeyThanks for the review, good that you spotted that mistake.
Fixed and committed.