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: Dave Hall
Subject: [Phpgroupware-tracker] [patch #2916] Accounts <-> Contact link fix
Date: Mon, 07 Jun 2004 11:34:22 -0400
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040602 Firefox/0.8

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

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

Changes by: 
                Dave Hall <address@hidden>
'Date: 
                Mon 06/07/2004 at 15:24 (Australia/Melbourne)

            What     | Removed                   | Added
---------------------------------------------------------------------------
         Assigned to | skwashd                   | fipsfuchs


------------------ Additional Follow-up Comments ----------------------------
fips,

you now have something to apply to the API ;)






/**************************************************************************/
[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/2004 at 03:29

Category:  API -  phpGWapi
Priority:  9 - Immediate
Resolution:  None
Assigned to:  fipsfuchs
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: Mon 06/07/2004 at 15:24       By: skwashd
fips,

you now have something to apply to the API ;)

-------------------------------------------------------
Date: Mon 06/07/2004 at 15:09       By: Caeies
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

-------------------------------------------------------
Date: Fri 06/04/2004 at 17:13       By: Caeies
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


-------------------------------------------------------
Date: Wed 06/02/2004 at 11:59       By: fipsfuchs
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 ;-)


-------------------------------------------------------
Date: Fri 05/14/2004 at 01:26       By: skwashd
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>

-------------------------------------------------------
Date: Thu 05/06/2004 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/2004 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/2004 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/2004 at 10:00       By: skwashd
This doesn't work :(




CC List
-------

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



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

-------------------------------------------------------
Date: Mon 06/07/2004 at 15:09  Name: patch_contact_account.diff  Size: 12KB   
By: Caeies
Patch against the api AND the admin app.
http://savannah.gnu.org/patch/download.php?item_id=2916&amp;item_file_id=3346

-------------------------------------------------------
Date: Wed 06/02/2004 at 11:57  Name: api-inc.patch  Size: 9KB   By: fipsfuchs
api part
http://savannah.gnu.org/patch/download.php?item_id=2916&amp;item_file_id=3335

-------------------------------------------------------
Date: Wed 05/05/2004 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]