Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
This is a sub-issue of #1775842: [meta] Convert all variables to state and/or config systems. This issue will move variable 'tracker_index_nid' to the state system.
Tasks
- Move variable_get/_set/_delete to state()->get/set/delete equivalents.
- Provide upgrade path.
Comments
Comment #1
Lars Toomre CreditAttribution: Lars Toomre commentedHere is a locally untested patch that addresses the above.
Comment #3
Lars Toomre CreditAttribution: Lars Toomre commentedLooks like misspelling tracker_update_8002() does not help... Here is a revised patch.
Comment #5
Lars Toomre CreditAttribution: Lars Toomre commentedMissed the use in TrackerTest.php... Included in this patch.
Comment #6
BerdirUpdate function needs to be updated to use the new helper, but looks good otherwise.
Comment #7
brandenlhamilton CreditAttribution: brandenlhamilton commented#6. The patch included in #5 is actually converting the variable to the State API, and uses a different upgrade path from the Config API. However, I did notice that the variable_del() function was still being used in an hook_uninstall implementation. Attached patch removes the hook as well as the function call.
Comment #8
BerdirYes, and there is a helper function to convert state variables now too, look for update_variables_to_state() :) That document needs to be updated to reflect that...
We should also add an upgrade test, see StateSystemUpgradePathTest, you just need to add it to drupal-7.state.system.database.php and that test class.
Comment #9
brandenlhamilton CreditAttribution: brandenlhamilton commented@Berdir thanks for giving me the name of the new helper function. For reference, the new function can be found at api.drupal.org. Nevertheless, for completeness, I also updated the original documentation page that I referenced in #7.
I took your suggestion, and added the tracker.index_nid to the upgrade test in StateSystemUpgradePathTest.php and drupal-7.state.system.database.php. Please let me know if I was mistaken in that interpretation.
The attached patch includes the updated state system test, as well as the changes implemented in #7.
Comment #11
brandenlhamilton CreditAttribution: brandenlhamilton commentedSeems my branch was a bit stale. Fetch/Merge latest and generate new patch.
Comment #13
brandenlhamilton CreditAttribution: brandenlhamilton commentedAck! Trying again.
Comment #14
BerdirLooks good.
Comment #15
catchShouldn't this delete the value from state now?
Comment #16
BerdirYes, it should. Thanks :)
Comment #17
Berdir#16: tracker-index-nid-1813184-16.patch queued for re-testing.
Comment #18
longwaveThe most nitpicky thing I think I've ever posted:
There is an unnecessary space after
key(
Comment #19
aspilicious CreditAttribution: aspilicious commentedNeeds a reroll based on #18
Comment #20
ACF CreditAttribution: ACF commentedJust a reroll of berdir's patch with tiny fix.
Comment #22
ACF CreditAttribution: ACF commented#20: 1813184-tracker_index_to_state-drupal8-20.patch queued for re-testing.
Comment #24
ACF CreditAttribution: ACF commented#20: 1813184-tracker_index_to_state-drupal8-20.patch queued for re-testing.
Comment #25
Lars Toomre CreditAttribution: Lars Toomre commentedComment #26
longwaveThe function comment for tracker_cron() needs updating, as it still mentions the 'tracker_index_nid' variable.
Comment #27
ACF CreditAttribution: ACF commentedupdated comment.
Comment #29
ACF CreditAttribution: ACF commented#27: 1813184-tracker_index_to_state-drupal8-27.patch queued for re-testing.
Comment #30
aspilicious CreditAttribution: aspilicious commentedLooking good, will retest to verify
Comment #31
aspilicious CreditAttribution: aspilicious commented#27: 1813184-tracker_index_to_state-drupal8-27.patch queued for re-testing.
Comment #32
webchickCommitted and pushed to 8.x. Thanks!
Comment #33.0
(not verified) CreditAttribution: commentedUpdated issue summary.