Closed (won't fix)
Project:
Entity API
Version:
7.x-1.x-dev
Component:
Entity CRUD admin UI
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
16 Mar 2011 at 21:08 UTC
Updated:
21 Mar 2011 at 09:14 UTC
I had a failing test when checking an upgrade path in OG. Anyway, this patch adds a class_exist() just to make sure we return a valid class.
| Comment | File | Size | Author |
|---|---|---|---|
| class-check.patch | 742 bytes | amitaibu |
Comments
Comment #1
fagoI'm not sure about that. Usually it's better to show the errors, so you know what's going wrong.
I don't know why this gets called during your OG upgrade though. Maybe you are you calling any functions during your upgrade? You shouldn't use your module's API in update functions.
Comment #2
amitaibuI'm adding a new entity for "group membership", and without this patch it crashes the test, and I couldn't find a workaround for this.
I also think being careful with creating a non existent class is a better practice then having a scary error page ;)
> You shouldn't use your module's API in update functions.
Normally yes, but here I have custom logic. so I add a
Comment #3
fagoI guess your "custom" logic is the cause of the error then. I know it's sometimes weird not being able to do that, but you really shouldn't as you cannot predict the state your module is when the update is run. It might even run some time in future when the called functions do not exist any more, or it might call any code that assumes the module is there and properly working already.
>I also think being careful with creating a non existent class is a better practice then having a scary error page ;)
Not really - if you are developing and something is wrong, it's important to see the error. If the system is just silently ignoring it, determining what's wrong is much harder.
Comment #4
amitaibu> I guess your "custom" logic is the cause of the error then
Hmm, The problem is that I don't know if I have other options -- og_update_7000() if for migrating from D6 to D7. and og_update_7001() if for a field schema change and creating group membership entity instead. Apart of hook_update_N() where can I deal with this?
Comment #5
amitaibu> og_update_7000() if for migrating from D6 to D7
This can actually be another module like CCK is doing, but not og_update_7001() that is moving data from the field into an entity.
btw, this is why I have tests for the update hooks :)
Comment #6
fago>This can actually be another module like CCK is doing, but not og_update_7001() that is moving data from the field into an entity.
So what about bypassing the entity API for writing the entity? I'm sry, but I don't think you can rely on a working entity API module in that state.
Comment #7
amitaibu> I'm sry, but I don't think you can rely on a working entity API module in that state.
No need to be sorry, you are helping me figure out a solution - This is an issue that I might be the first to encounter, but I;m sure other will have -- the need to move data from a field to another place. (I wish I would have thought about "group membership" as an entity sooner...). If you have an idea how to do it without hook_update_N I'd be happy to hear.
Anyway, I'll try to find a workaround...
Comment #8
fagoIf it is about updating field-data, shouldn't you use content-migrate? Or does that only work for field-field updates? Else yep, a seprate "update data" screen/functionality would be a way to go too.
Comment #9
amitaibubtw, the issue is not with the upgrade itself, it just the *test* if the update, so it's not a blocker.
Marking as won't fix.
Comment #10
amitaibuHi fago,
Just wanted to let you know I've taken your advice and decided to move all the custom code into "og-migrate" module. Still working on it, but no doubt it's the right approach. Thanks!