phpgroupware-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Phpgroupware-tracker] [patch #2916] Accounts <-> Contact link fix


From: Philipp Kamps
Subject: [Phpgroupware-tracker] [patch #2916] Accounts <-> Contact link fix
Date: Thu, 06 May 2004 04:14:57 -0400
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031107 Debian/1.5-3

This mail is an automated notification from the patch tracker
 of the project: phpGroupWare.

/**************************************************************************/
[patch #2916] Latest Modifications:

Changes by: 
                Philipp Kamps <address@hidden>
'Date: 
                Thu 05/06/04 at 08:14 (GMT)

------------------ Additional Follow-up Comments ----------------------------
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.







/**************************************************************************/
[patch #2916] Full Item Snapshot:

URL: <http://savannah.gnu.org/patch/?func=detailitem&item_id=2916>
Project: phpGroupWare
Submitted by: Dave Hall
On: Thu 04/08/04 at 03:29

Category:  API -  phpGWapi
Priority:  9 - Immediate
Resolution:  None
Assigned to:  skwashd
Originator Email:  
Status:  Open


Summary:  Accounts <-> Contact link fix

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

Follow-up Comments
------------------


-------------------------------------------------------
Date: Thu 05/06/04 at 08:14         By: fipsfuchs
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.


-------------------------------------------------------
Date: Wed 05/05/04 at 18:07         By: alexbsa
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?






-------------------------------------------------------
Date: Wed 05/05/04 at 13:59         By: fipsfuchs
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


-------------------------------------------------------
Date: Sat 05/01/04 at 10:00         By: skwashd
This doesn't work :(




CC List
-------

CC Address                          | Comment
------------------------------------+-----------------------------
charlesk --AT-- generalpants --DOT-- com --DOT-- au | 
jarg                                | 
benoit --DOT-- hamet --AT-- int-evry --DOT-- fr | FYI



File Attachments
-------------------

-------------------------------------------------------
Date: Wed 05/05/04 at 13:59  Name: phpgwapi-inc.patch  Size: 10KB   By: 
fipsfuchs

http://savannah.gnu.org/patch/download.php?item_id=2916&amp;item_file_id=3204

-------------------------------------------------------
Date: Wed 05/05/04 at 13:49  Name: admin.patch  Size: 1KB   By: fipsfuchs

http://savannah.gnu.org/patch/download.php?item_id=2916&amp;item_file_id=3203






For detailed info, follow this link:
<http://savannah.gnu.org/patch/?func=detailitem&item_id=2916>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/







reply via email to

[Prev in Thread] Current Thread [Next in Thread]