Closed (fixed)
Project:
Commerce Addressbook
Version:
7.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
29 Mar 2012 at 19:07 UTC
Updated:
21 Apr 2012 at 19:31 UTC
Jump to comment: Most recent file
Comments
Comment #1
BassistJimmyJam commentedAttached a patch that should take care of it (unless I'm missing something).
Comment #2
BassistJimmyJam commentedAccidentally changed file mode and included some other changes in that patch that did not belong. New one is attached.
Comment #3
BassistJimmyJam commentedI didn't realize the saved profile field type had been removed. The attached patch adds removing all commerce address book saved profile fields to the update hook. This also resolved the "Undefined index: commerce_addressbook_saved_profiles" notices.
Comment #4
damien_vancouver commented+1 on the patch from #3.
Error messages are gone, so is the old field, and the commerce_addressbook_defaults table created successfully! Thanks so much!
This also solves #1502120: Undefined index: commerce_addressbook_saved_profiles, so I marked it as a duplicate of this issue.
Comment #5
summit commentedHi, +1 to commit patch from #3. Thanks.
greetings, Martijn
Comment #6
bojanz commentedThis can't work, because at that point the field type no longer exists (it was in Addressbook 1.x and you've replaced those files with Addressbook 2.x).
The issue that you actually want is #1311470: Provide missing uninstall code.
That will allow you to uninstall Addressbook 1.x, which will delete the fields.
Then, when you install 2.x, the defaults table will be created, and all will be well.
The flow needs to be like that (uninstall -> update -> install).
Comment #7
BassistJimmyJam commentedSince the field type name is stored in the database with the field type definition, you can still get a list of all fields and look for that type. While hook_field_info is no longer available for the field type, this did not seem to introduce any issues. Perhaps the field type definition was still cached which could potentially present a problem if the cache is cleared before the update hook is run. I could change the update hook to use db_select() since all I really need is the field name. Deleting the field does not call any field type specific hooks so there is no need to be concerned about the field type no longer existing at that point.
I don't see any reason to force people to go though the rigmarole of disable -> uninstall -> update -> install when such a simple update hook could be introduced to accomplish the same task. Especially when that update hook has seen success from not only myself, but other community members as well.
Comment #8
bojanz commentedHere's some background information for you: #943772: field_delete_field() and others fail for inactive fields.
So yes, it works, but the field is still there with all its data, it has just been marked as inactive so it's not shown. Spending 5 minutes more (on reinstalling) to ensure there's no garbage data left in the database seems like a fair tradeoff to me.
Comment #9
BassistJimmyJam commentedThanks for pointing me that issue. It looks like the issue was resolved in 7.8 and my testing was on 7.12 so I did not experience this behavior. If this were a perfect world I would still argue for an upgrade path, but it's not and I can understand being apprehensive about including a change that's dependent on core version. I will leave this to your discretion.
Comment #10
bojanz commentedIt is resolved if you don't provide a field type AND create instances of that type in the same module, which is exactly what Addressbook did.
So because of that you can't disable Addressbook 1.x in the first place
However, I was wrong in thinking that's a problem, since you can just replace the files with 2.x ones without disabling the module.
So let's give it a shot anyway.
Here's what needs to be done:
1) Only do the update if the defaults table doesn't exist already (so that it doesn't run for existing 7.x-2.x users)
2) Remove the field_purge call, it's useless.
3) Replace the foreach (field_info_fields() as $field) { loop with a call to commerce_delete_fields('commerce_addressbook_saved_profiles');
4) See if it works.
Can you do a reroll?
Comment #11
BassistJimmyJam commentedAbsolutely, I'll get to work on it right away.
Comment #12
BassistJimmyJam commentedPatch attached with changes based on #10.
Comment #13
fakingfantastic commented+1 on #12 patch. I was getting Call to undefined function commerce_addressbook_field_is_empty(). Applied patch the .install and updated the db. All cleaned up.
Comment #14
bojanz commentedAdded information to the README and committed the patch. Thank you!