patchphpGroupWare - Patches: patch #2916, Accounts <-> Contact link fix

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #2916: Accounts <-> Contact link fix

Submitter:  Dave Hall <skwashd>
Submitted:  Thu 08 Apr 2004 03:29:06 AM UTC
   
 
Category:  API - phpGWapi Priority:  9 - Immediate
Item Group:  None Status:  Done
Privacy:  Public Assigned to:  fipsfuchs
Open/Closed:  Closed

Jump to the original submission

Fri 18 Jun 2004 09:14:40 AM UTC, comment #18: 

The bug was in admin module - should be fine now for accounts_ldap and accounts_sql. Plz reopen if not.

Philipp Kamps <fipsfuchs>
Fri 18 Jun 2004 08:30:21 AM UTC, comment #17: 

I can confirm this for account_ldap. account_sql doesn't have this problem. I'll have a look.

Philipp Kamps <fipsfuchs>
Wed 16 Jun 2004 06:56:19 PM UTC, comment #16: 

dam, this break the "Edit entry" link,

as admin user which is addressmaster:

1.create a new user

the link "Edit entry" is ok

2.click in "Edit entry" link and edit the contact and save

3.click in done in view contact screen

now in Edit Account screen i have "This account has no contact entry yet" , really my account now haven't person_id in phpgw_accounts

i think your code lost the person_id in the update.

the code which you drop haven't this problem.

i open this patch again, but do you want that i open a new bug?

Jonathan Rivera <jarg>
Wed 16 Jun 2004 06:55:34 PM UTC, comment #15: 

dam, this break the "Edit entry" link,

as admin user which is addressmaster:

1.create a new user

the link "Edit entry" is ok

2.click in "Edit entry" link and edit the contact and save

3.click in done in view contact screen

now in Edit Account screen i have "This account has no contact entry yet" , really my account now haven't person_id in phpgw_accounts

i think your code lost the person_id in the update.

the code which you drop haven't this problem.

Jonathan Rivera <jarg>
Tue 15 Jun 2004 01:24:40 PM UTC, comment #14: 

tested with sql and ldap
committed

Philipp Kamps <fipsfuchs>
Tue 15 Jun 2004 11:06:13 AM UTC, comment #13: 

The small bug for Edit Entry is true in some case only. I get it working sometimes, and sometimes not. I need to look at it to change the order to proceed, and then solve this little problem.

Caeies <Caeies>
Group administrator
Tue 08 Jun 2004 01:39:27 PM UTC, comment #12: 

@jarg:

if the call for save_contact_for_account stays in the account logic every class have to implement this logic. if it moves to admin the accounts backend don't have to care about it. In the unlikely case, another module (not admin) wants to create an account it has two possibilities:

it useses directly the accounts backend -> no link to contacts

it useses the admin application -> all extra logic like account <-> contact link will happen.

Philipp Kamps <fipsfuchs>
Tue 08 Jun 2004 05:31:47 AM UTC, comment #11: 

... and this patch only have a small bug, when you create a new account the "Edit entry" link are not showed immediately.

the current code have not this problem.

Jonathan Rivera <jarg>
Tue 08 Jun 2004 05:18:07 AM UTC, comment #10: 

heya

i have a question here, why remove save_contact_for_account call from the api and put this in admin?, i put this code in api (save_repository and create functions) because the link account-contact would be always that you save account data and if you put this in admin only admin do it, so if i use api functions never have the link account-contact.

Regards

Jonathan Rivera <jarg>
Mon 07 Jun 2004 03:24:37 PM UTC, comment #9: 

fips,

you now have something to apply to the API ;)

Dave Hall <skwashd>
Group administrator
Mon 07 Jun 2004 03:09:44 PM UTC, comment #8: 

Ok, I got it :)

That's my patch for the fix, It's based on fips/skwashd work.

Some comments :

I remove another call to save_contact_for_account in class.account_sql.inc.php (in save_repository). Perhaps not needed, should to be tested.

I add a call to save_contact_for_account when saving users.
so If a user had no link and is already existing, it will be created, and data are uptaded accordingly to the changes (Hope that don't break things). This need to be tested against SQL accounts.

Due to this behavior, I change the way that create_account works in the case we need to modify the objectclass (from phpgwAccount to phpgwContact). So I will delete the entry, and recreate it with appropriate values.

I add a bugfix (sorry) for the get_account_data (renaming fields accordingly to the new system).

I think that the patch need to be reworked because I left many comments in it.

Regards,
Caeies

Caeies <Caeies>
Group administrator
Fri 04 Jun 2004 05:13:15 PM UTC, comment #7: 

Hi all,
That's some answer.

The solution (Yes It's working for me :) is not "simple"

Fips, you need to modify read_repository()(in account_ldap) to add the person_id, so they will be retry when they exists

Then you need to modify edit_user (in admin.boaccount) so That it a person_id is not set create one.

I can't give you a patch because I take me too many time to do it this evening (I'm late :).

And I correct more bug in LDAP (with get_account_data())
and add the quota support to it (At least it's saved and restored in the admin, I don't know if it works with filemanager).

Hope that I'm enough clear to help you.
I will sent to fips a full patch in few minutes, but things are total mess. If you find the info use them, If not, wait for a better patch :)

Cya
Caeies

Caeies <Caeies>
Group administrator
Wed 02 Jun 2004 11:59:43 AM UTC, comment #6: 

hoho,

as far i see the patch fails for the api part NOT for the admin part. So i replaced the patch for the api part. I tested the new patch ONLY with accounts_sql - benoit plz have a quick test with accounts_ldap - thank you.

btw: savannah is fast like shit! - makes a lot of fun working with it ;-)

Philipp Kamps <fipsfuchs>
Fri 14 May 2004 01:26:09 AM UTC, comment #5: 

I have no idea what is happening with the changes in the api part of this patch.  It fails to apply and looking at the rej file, it looks like it removes the code to handle this.

Can I please get some more info. 

<rant>
Also if someone has applied part of it - YOU IDIOT - apply the whole thing or not at all.
</rant>

Dave Hall <skwashd>
Group administrator
Thu 06 May 2004 08:14:54 AM UTC, comment #4: 

Hi Lex,

do you mind to move this discussion to the dev-list?

You're right - admin is not the right place of the sync stuff.  But the accounts backend is worse in my opinion. The best solution would be to have a logic in the api to combine/link phpgw elements (not only accounts<->contacts). Dirk is actually working on this.

If you look to the resulting code of my patch. The sync logic (method sync_accounts_contacts(), get_account_with_contact(), get_account_without_contact()) are still in the accounts backend.
I only moved the method call from !!class.accounts_SQL.inc.php!! the the admin interface - which makes a lot more sense.

But I'm not very happy having the sync logic still in the accounts backend - because the sync (accounts<->contacts) is separated thing. I love to keep things simple and general. It allows other people to understand the code much more quicker.

Philipp Kamps <fipsfuchs>
Wed 05 May 2004 06:07:20 PM UTC, comment #3: 

Okok...

I think its great that we can have this thing working with ldap. It really could complete all that is missing.

One thing though.... This patch removes the account-contact link code out of the api and moves it into admin.

Even if it is the least effort path to do this, its not necesarily the best idea.

For one, admin is very UI tied. Its classes are hardly reusable from other applications.  So, if you were to make a script that bulk-added accounts, it would be harder to do it using the code as it is put by this patch in admin.

Another thing is that the patch removes the "sync accounts with contacts" HACK  in the admin section. It may be so that it is not usefull if the account/contact scheme is working perfectly, but it was there as safeguard just in case we messed up or some legacy code did something wrong (like import/export in early stages).

But now, if you take the account/contact sync code out of accounts, and into admin, then there is a path to create accounts without a corresponding contact is open (ExecMethod('phpgwapi.accounts.create_account');).

So perhaps the sync hack will be usefull if we accept how this patch wants  to move the sync code into admin.

So i guess my only grudge with this is: Why is it not a good idea to do the contacts syncing in the accounts_ldap and accounts_sql classes?





Alejandro Borges (lex) <alexbsa>
Wed 05 May 2004 01:59:30 PM UTC, comment #2: 

so boys and girls,

this one was tricky - but i got it finally:

- the creation of the contact for the account was in class.accounts.inc.php. I moved this logic to the admin module; so it is working with ldap as well. Some superfluous code could be removed therefore.

- class.accounts_ldap.inc.php know saves the person_id of this new contact in ldap. I added the phpgwcontact objectclass to the ldap entry; it has the field phpgwcontactid which is been used to store the value.

- i discovered that the phpgwcontact objectclass is structural and not auxiliary. This isn't good at all and i'll have to change the schema file for HEAD.

- i disabled the account<->contacts sync thing in the admin interface (ONLY for LDAP). It doesn't do anything usefull right now.

- if you have time, plz test the patch and give me some feedback.

Thanks, fips

Philipp Kamps <fipsfuchs>
Sat 01 May 2004 10:00:21 AM UTC, comment #1: 

This doesn't work :(

Dave Hall <skwashd>
Group administrator
Thu 08 Apr 2004 03:29:06 AM UTC, original submission:  

Hi

I have found that when the new ldap schema was done, the accounts<->contacts link was removed.  This patch is an attempt to fix it.

I know you can't apply it, I just hope you can tell me if it is ok.

Cheers

Dave Hall <skwashd>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #7537:  patch_contact_account.diff added by Caeies (12KiB - text/x-patch - Patch against the api AND the admin app.)
file #7387:  api-inc.patch added by fipsfuchs (10KiB - text/x-patch - api part)
file #7254:  admin.patch added by fipsfuchs (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

CC list is empty

 

There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

 

Follow 25 latest changes.

Date Changed by Updated Field Previous Value => Replaced by
2004-06-18 fipsfuchs Open/ClosedOpen Closed
2004-06-16 jarg Open/ClosedClosed Open
2004-06-15 fipsfuchs StatusNone Done
    Open/ClosedOpen Closed
2004-06-15 Caeies Carbon-Copy- Added caeies
2004-06-15 Caeies Carbon-CopyRemoved benoit --DOT-- hamet --AT-- int-evry --DOT-- fr -
2004-06-07 skwashd Assigned toskwashd fipsfuchs
2004-06-07 Caeies Attached File- Added patch_contact_account.diff, #3346
2004-06-02 harriv Carbon-Copy- Added harriv
2004-06-02 fipsfuchs Attached File- Added api-inc.patch, #3335
2004-06-02 fipsfuchs Attached File#3204 Removed
2004-05-05 fipsfuchs Attached File- Added phpgwapi-inc.patch, #3204
2004-05-05 fipsfuchs StatusInvalid None
    Assigned tofipsfuchs skwashd
    Open/ClosedClosed Open
    Attached File- Added admin.patch, #3203
2004-05-05 fipsfuchs Attached File#3077 Removed
2004-05-05 fipsfuchs Attached File#3075 Removed
2004-05-01 skwashd StatusNone Invalid
    Open/ClosedOpen Closed
2004-04-08 skwashd Carbon-CopyRemoved charliek --AT-- generalpants --DOT-- com --DOT-- au -
2004-04-08 skwashd Carbon-CopyRemoved jarg --AT-- sogrp --DOT-- com -
2004-04-08 skwashd Carbon-Copy- Added charlesk --AT-- generalpants --DOT-- com --DOT-- au
    Carbon-Copy- Added jarg
2004-04-08 skwashd Carbon-Copy- Added jarg --AT-- sogrp --DOT-- com

Back to the top

Powered by Savane 3.13-758e.
Corresponding source code