Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
jibran in #2528178-172: Provide an upgrade path for blocks context IDs #2354889 (context manager) suggested adding a test module that does a contrib module update for block contexts.
This would ensure it's actually possible for the test module to do the update.
Extra points would be for the update to handle and be tested for the two different scenarios it can run in:
In between block_update_8001() and block_update_8002()
After block_update_8002() has run (which will happen if a contrib modules adds an update hook after the core update has already run on a site).
Comments
Comment #1
BerdirSo I was trying to implement this for a custom module in my project and it doesn't work..
The problem is that we drop the whole visibility condition but only store the missing context mappings in the backup values. So a module has no chance to actually restore the whole thing.
I don't think this is a critical problem since as far as I know, I'm only case I know of where someone actually wrote a block context that needs to be upgraded (not aware of any contrib module doing it), but we should probably update it and make this test work to have a good example of how it should be done.
I can see two ways to do that.
a) The easy limited way: Move the removal of visibility conditions to 8002(). If there is a module that provides a mapping in the meantime and removes it from backup_values, then we don't need to drop it and can just move on. The limitation is that this won't work for updates that run after 8002, so we should be honest and just drop the backup values in 8002. In case a module update runs later, it will just be a no-op and the user will have to figure it out manually (probably already has).
b) Store the full visibility condition in the backup values. While this sounds like an obvious choice, at first, it actually not that easy. First, it would be quite complicated to implement correctly if there are multiple contexts from different modules in the same condition. Second, as discussed, it would be quite hard to write an update function that would work both between block 8001 and 8002 and after and I'm not actually sure that they are still supposed to run after 8002. If it didn't run in the same update, then we have to assume that the user already manually reconfigured is blocks.
Given that, I think a) is the correct way to go about this.
Comment #2
dawehnerAre you sure that this is just a major issue? It sounds for me to block the update path of an actual contrib module, right?
Comment #3
BerdirIt blocks the update of such a module, but I very much doubt that any other module other than mine exists and the worst case is that users will have to manually re-configure their blocks.
Comment #15
quietone CreditAttribution: quietone at PreviousNext commentedIs there anything to do here?
In #1, @Berdir states that they were not able to implement a test. And later, in #3, that the "worst case is that users will have to manually re-configure their blocks."
Comment #16
quietone CreditAttribution: quietone at PreviousNext commentedThere has been no response to confirm that there is anything to do here. Therefor I am closing this as works as designed.