Re: [evolution-patches] Patch for implementation of modify Cal objects in GW backend

[Rodrigo Moya <rodrigo ximian com>]

On Wed, 2004-05-12 at 16:00 +0530, Harish Krishnaswamy wrote:
> hi,
> 
> the patch contains code that allows modification of calendar objects
> using the modifyItemRequest of the GW SOAP interface instead of the
> sendItems call.
> 
> Also included is a fix for letting evo set the default timezone on the
> GW backend.

> +e_gw_item_set_changes (EGwItem *item, EGwItem *cache_item)
> +{
> +	char *subject, *cache_subject;
> +	char *message, *cache_message;
> +	char *classification, *cache_classification;
> +	char *accept_level, *cache_accept_level;
> +	char *place, *cache_place;
> +	char *priority, *cache_priority;
> +		
> +	/* TODO assert the types of the items are the same */
> +
> +	SET_DELTA(subject);
> +	SET_DELTA(message);
> +	SET_DELTA(classification);
> +
> +	if (difftime (e_gw_item_get_start_date (item),
> e_gw_item_get_start_date (cache_item)))
> +		e_gw_item_set_change (item, E_GW_ITEM_CHANGE_TYPE_UPDATE,
> "startDate", e_gw_item_get_start_date (item));
> +	if (difftime (e_gw_item_get_end_date (item), e_gw_item_get_end_date
> (cache_item)))
>
end_date should only be used for APPOINTMENTS, right?

> +		e_gw_item_set_change (item, E_GW_ITEM_CHANGE_TYPE_UPDATE, "endDate",
> e_gw_item_get_end_date (item));
> +
> +	if ( e_gw_item_get_item_type (item) == E_GW_ITEM_TYPE_APPOINTMENT) {
> +		accept_level = e_gw_item_get_accept_level
> (item);                                                       
> +		cache_accept_level = e_gw_item_get_accept_level
> (cache_item);                                           
> +		if ( cache_accept_level )
> {                                                                            
> +			if (!accept_level
> )                                                                               
> +				e_gw_item_set_change (item, E_GW_ITEM_CHANGE_TYPE_DELETE,
> "acceptLevel", cache_accept_level );
> +			else if (strcmp ( accept_level, cache_accept_level
> ))                                               
> +				e_gw_item_set_change (item, E_GW_ITEM_CHANGE_TYPE_UPDATE,
> "acceptLevel", accept_level );
> +		}                                                                                                 
> +		else if ( accept_level
> )                                                                               
> +			e_gw_item_set_change (item, E_GW_ITEM_CHANGE_TYPE_ADD,
> "acceptLevel", accept_level ); 
> +		
> +		SET_DELTA(place);
> +		if ( e_gw_item_get_trigger (cache_item) )
> {                                                                            
> +			if (!e_gw_item_get_trigger (item)
> )                                                                               
> +				e_gw_item_set_change (item, E_GW_ITEM_CHANGE_TYPE_DELETE, "alarm",
> e_gw_item_get_trigger (cache_item));
> +			else if (e_gw_item_get_trigger (item) != e_gw_item_get_trigger
> (cache_item))                                               
> +				e_gw_item_set_change (item, E_GW_ITEM_CHANGE_TYPE_UPDATE, "alarm",
> e_gw_item_get_trigger (item));
> +		}                                                                                                 
> +		else if ( e_gw_item_get_trigger (item)
> )                                                                               
> +			e_gw_item_set_change (item, E_GW_ITEM_CHANGE_TYPE_ADD, "alarm",
> e_gw_item_get_trigger (item)); 
> +	}
> +
> +	if ( e_gw_item_get_item_type (item) == E_GW_ITEM_TYPE_TASK) {
>
instead of using an 'if' for each item type, it would be better to use a
if/else if/else, or even better a switch statement.

apart from that it looks ok

cheers