I have an issue, regarding GalleryEmbed::init(), regarding 'activeUserId'.
In the WP/G2 data model not all the WP Users will be replicated in G2, mostly because user registration is a common requirement to get around mass spam in WP blogs thus why replicate hundreds of WP users in G2 when you have no real requirement to do so. This will lead to a situation where a user is logged into WP yet be treated as a guest in G2.
This concept is fine in the G2 data model except in the case of GalleryEmbed::init() calls, specifically in passing a value in 'activeUserId' as by default the GalleryEmbed::init() function tries to verify that the user exist in G2, otherwise it returns an error.
Now to get around this, you have to make a series of calls like the following..
GalleryEmbed::init() as WP User
If GalleryEmbed::init() fails, reinitalise GalleryEmbed::init() as Guest
If continues to fail, return error..
I rather just make one GalleryEmbed::init() call if the user does not exist then the session be treated as guest.
Can the GalleryEmbed::init() function call please be changed to treat the activeUserId as a guest if it does not match an existing G2 external User.
Posts: 32509
I'd say most embedded G2 setups would create a G2 user when someone visits embedded G2 and the user doesn't already exist. creating a G2 user isn't a bad thing...
And you never have to call init twice. I thought we have discussed this with the on the fly user creation.
When init fails, check isextidmapped, then create the user (obviously you don't wanna do this?!), then call ::checkActiveUser (not init).
I guess you wanna create G2 users only for those WP users that are in a certain WP group / level.
Right? Wouldn't it then be easier to check if the WP user should be created / exist in G2 before you call ::init?
And then set activeUserId => $uid to the real WP user id if this user should have a corresponding WP user and set it to '' (empty string) if not.
Posts: 1378
Maybe I missing something here, but checkActiveUser or isextidmapped both require GalleryEmbed::init to be active in the first place, thus..
Step 1. GalleryEmbed::init()
Step 2. isExternalIdMapped to check if the user exists
Step 3. GalleryEmbed::init() as a the mapped user..
Which brings me right back to my first point, you could reduce this down to one step by one GalleryEmbed::init() call passing the userid and if it is not mapped, G2 treats it as a guest..
Posts: 32509
no.
as a programmer, you can read the source code for yourself ;)
modules/core/classes/GalleryEmbed.class
-> function init and function checkActiveUser.
init does nothing else than call GalleryInitFirstPass from init.inc and then checkActiveUser from GalleryEmbed.class.
So After ::init returned an error, you already called GalleryInitFirstPass.
-> all you need to call then is checkActiveUser, calling GalleryInitFirstPass a second time would be inefficient and is not required.
bottom line: you will never have to call GalleryEmbed::init more than once in the same request.
but back to your first post in this topic (and my first answer).
have i interpreted your motivation correctly? and is my approach not ok? there's no point in calling GallerEmbed::init with an activeUserId unless you really want that a corresponding user exists in G2. If you wish that no such user is created in G2, set activeUserId to ''.
To your other request:
Why GalleryEmbed::init doesn't create a G2 user for an activeUserId if it doesn't find a mapping...
Because when we create a G2 user for an externalId, we'd like to populate the user with a username, (and other user data). At the same time, we'd like to keep the load as low as possible / the performance as fast as possible. Querying the emApp for user data is considered as something that is not for free. the activeUserId aka the uid in emApp is considered to be fairly simple. But to get the email address, you often have to query the db.
And GalleryEmbed::init would get overly complex (yet more parameters).
If you have (another) improvement how the (on-the-fly) user creation could be simplified, i.e. by moving some code into GalleryEmbed, let me know.
Posts: 1378
Actually I am thinking maybe that GalleryEmbed::init should not support activeUserId as should the user not exist then it will fail which is not a good thing as you need in the class active to be able to do any API calls even as guest..
After a long talk with Bharat I am just going to implement the following (keeping in mind we do not replicate all WP users as G2 Users)..
Check to see if GalleryEmbed::init has not already been called, if not..
1. GalleryEmbed::init
2. Check WP user has been mapped via the isExternalIdMapped
3. If Mapped, then checkActiveUser
Hope this logic is fine with you Valiant
Posts: 1378
hmmm I would have thought this would have worked, but I cannot access gallery photos when I utilise this code..
Posts: 32509
ozgreg, I sent you an email in the hope to resolve our issues
I've taken a few notes from your irc discussion with bharat:
I agree. You've already created a thread for this topic, let's discuss it there.
Honestly, I thought we could call GalleryEmbed::init as many times as we wish. But I see now that this could lead to problems.
But this is another topic, let's discuss it somewhere else. I have to ponder a few things concerning db transactions and such.
That works, yes. But I'd like to discuss that a little more indepth. I'd like to understand what you want to achieve, because I don't understand it at the moment.
Here's what I think you'd like to do:
- Don't create G2 users for all WP users.
My take on this: That's ok. I see two possible usages of GalleryEmbed. Either sync' all users or only a subset. You choose the later.
- Somehow decide which WP users should have a G2 account and which not.
My take: How do you decide which WP users should have a G2 account and which should have an anonymous session in G2?
I'm not talking about the Guest/Anonymous user of WP, because the guest user is never synced with G2 of course.
Do you have something like a group / level in WP, and all users that are in this group, should have a G2 account?
I'll explain why you don't have to keep track of the sync status in WP:
- If all WP users are synced with hooks (create hook, ...), then you don't need any function to check if a user is already mapped / synced.
- If all WP users are synced with the method "call init and then check for errors and check if isExternalIdMapped)", then you only need isExternalIdMapped because we keep track of this list in G2 and isExternalIdMapped is our interface to this list.
- If only a subset of WP users are synced, you need in addition to isExternalIdMapped() another list. A list of all WP users that should have / receive a G2 account. Something like a WP user group "G2users".
Then you could check if a WP user is in this WP group, and if so, call GalleryEmbed::init() with his WP user id. If he is not in this group, call GalleryEmbed::init with activeUserId = '' as you would if the active WP user was a guest user.[/]
[/]
[/]
[/]
Posts: 1378
Thanks for your reply & email valiant, will answer both after some shuteye..
I just implemented the changes Bharat recommended although it has broken the plugin but I handball the changes over for review see what I overlooked..
Posts: 32509
ozgreg, @your code snippet:
- why do you use the "wp_" prefix for the WP user id when syncing with G2? I ask because it isn't required, it works also without such a prefix.
It would only be required if you plan to sync multiple CMS with a single instance of G2.
I've worked as a database administrator / designer for a while and one of the things I've learnt is that there shouldn't be any information in the (primary) key / or id fields.
However, if you just prefer to have the wp_ prefix, that's ok.
- I wouldn't recommend the usage of GalleryEmbed as in this snippet. The single reason is performance.
In this snippet, you call GalleryEmbed::isExternalIdMapped() on each embedded G2 request for all non-anonymous WP users.
This translates to a db query and we'd like to keep the total nr. of (unnecessary) db queries as low as possible.
- I don't know why your code snippet doesn't work.
But you should check the result of the GalleryEmbed::checkActiveUser() call ($ret->isError()).
I guess I could answer your questions much better if I understood your goal better. So please take the time and explain it to me (see my post from before).
Posts: 1378
Found the silly bug (I forgot to return the error status, even although it was false) which is why the snippet was broken..
The WP_ prefix, it is not actually my idea but I really like it as should someone want to run more than one embedded application utilising the same Gallery DB then our plugin will support this concept..
Technically we should be only calling the GalleryEmbed::checkActiveUser() once and once only per visit thus yes it adds one more read however it allows us to fully control the mapping of the WP/G2 users, should that be all or a subset..
Posts: 32509
ozgreg, please take the time to explain what your integration strategy is. Because, as i said, i still don't know if we understand us correctly. I'm quite sure I know what you want to achieve and that my proposed solution is cleaner/easier/faster and that you don't accept it because of misunderstandings. Perhaps I understand you wrong, that's why I ask. So here's my quoted post from before...
What I suggest:
1. get the WP user id
2. check if it's the guest user
3.1. if it's the guest user, call ::init with activeUserId = ''.
3.2. else { check if you this WP user should receive or already have a G2 user. i.e. by checking if this WP user is in a WP user group "wpg2users".
3.2.1. if so, call ::init with activeUserId = wpUserId
3.2.2. else { call ::init with activeUserId = ''
4. check if ::init returned an error
5.1 if so, if wp user is in wpg2users group, create wp user in G2
5.2 else { a real error
all i say is: calling ::init with activeUserId = '' and then checking with GalleryEmbed::isExternalIdMapped() if he should be rather logged in with a userId, is slow.
It's your integration. I just recommend a usage that is faster, which leads to more pleased WPG2 customers. [/]
Posts: 1378
Implemented your suggestions and stopped the init:: from being called more than once..
Hope to move to the first cycle beta tomorrow.. (Provided I can get the documentation done tonight) ;)