GNOME Bugzilla – Bug 711563
Support PicasaWeb
Last modified: 2014-08-05 16:08:41 UTC
Importing and exporting images to Google+ may be a difficult task. However, it's possible to upload and download images directly from Google Drive. By adding support to Picasa and/or Google Drive, it becomes easy to link the desktop with Google+. Dropbox is a great alternative for Google Drive. Many people, myself included, transfer their photos from their phones or tablets to the desktop (or vice versa) via Dropbox. Adding Dropbox to the list of online sources would enhance the usability of GNOME Photos.
Lets take it one at a time. I am dropping the Dropbox part from this bug. Lets just use this one for PicasaWeb. We can use a separate bug for Dropbox.
Created attachment 280378 [details] [review] Add PhotosPicasaWebItem
Review of attachment 280378 [details] [review]: Thanks for the patch, Saurav. A few initial comments while testing bug 728878 ::: src/photos-picasaweb-item.c @@ +49,3 @@ + g_io_extension_point_implement (PHOTOS_BASE_ITEM_EXTENSION_POINT_NAME, + g_define_type_id, + "picasaweb", Should be 'google' to match the changes in bug 728878 ::: src/photos-picasaweb-item.h @@ +24,3 @@ + +#ifndef PHOTOS_PICASAWEB_ITEM_H +#define PHOTOS_PICASAWEB_ITEM_H We might as well call this PhotosGoogleItem. ::: src/photos-utils.c @@ +314,3 @@ + if (g_str_has_prefix (identifier, "facebook:") || + g_str_has_prefix (identifier, "flickr:") || + g_str_has_prefix (identifier, "picasaweb:")) Should be 'google:'.
Created attachment 281579 [details] [review] Add PhotosPicasaWebItem
Review of attachment 281579 [details] [review]: Thanks, Saurav. There are a few more s/picasaweb/google/ left, and some minor things. ::: src/Makefile.am @@ +157,3 @@ photos-overview-searchbar.h \ + photos-picasaweb-item.c \ + photos-picasaweb-item.h \ photos-google-item.[ch] ::: src/photos-application.c @@ +73,3 @@ GomMiner *facebook_miner; GomMiner *flickr_miner; + GomMiner *picasaweb_miner; google_miner or gdata_miner ::: src/photos-picasaweb-item.c @@ +58,3 @@ + PhotosPicasaWebItemPrivate *priv = PHOTOS_PICASAWEB_ITEM (item)->priv; + PhotosSource *source; + const gchar *identifier, *resource_urn; One variable per line, please. @@ +68,3 @@ + source = PHOTOS_SOURCE (photos_base_manager_get_object_by_id (priv->src_mngr, resource_urn)); + authorizer = gdata_goa_authorizer_new (photos_source_get_goa_object (source)); + identifier = photos_base_item_get_identifier (item) + strlen("google:picasaweb:"); Missing white space. @@ +94,3 @@ + GDataMediaThumbnail *thumbnail = NULL; + gchar *local_path = NULL, *local_dir = NULL; + GFile *local_file = NULL, *remote_file = NULL; One variable per line, please. @@ +126,3 @@ + + local_path = gnome_desktop_thumbnail_path_for_uri (photos_base_item_get_uri (item), + GNOME_DESKTOP_THUMBNAIL_SIZE_NORMAL); Our thumbnails are 160x160, aren't they? If that is the case, then I think we should use GNOME_DESKTOP_THUMBNAIL_SIZE_LARGE because the documentation specifies that _NORMAL is for sizes upto 128x128. @@ +176,3 @@ + cache_dir = g_get_user_cache_dir (); + + local_dir = g_build_filename (cache_dir, PACKAGE_TARNAME, "picasaweb", NULL); Let's use 'google' for consistency. @@ +237,3 @@ + const gchar *picasaweb_uri; + + picasaweb_uri = photos_base_item_get_uri (item); google_uri @@ +296,3 @@ + PhotosBaseItemClass *base_item_class = PHOTOS_BASE_ITEM_CLASS (class); + + object_class->constructed= photos_picasaweb_item_constructed; Missing white space. ::: src/photos-picasaweb-item.h @@ +54,3 @@ +typedef struct _PhotosPicasaWebItem PhotosPicasaWebItem; +typedef struct _PhotosPicasaWebItemClass PhotosPicasaWebItemClass; +typedef struct _PhotosPicasaWebItemPrivate PhotosPicasaWebItemPrivate; PhotosGoogleItem ::: src/photos-utils.c @@ +314,3 @@ + if (g_str_has_prefix (identifier, "facebook:") || + g_str_has_prefix (identifier, "flickr:") || + g_str_has_prefix (identifier, "google:picasaweb:")) Just 'google:' is enough. Using the name of the extension point will make it easier to refactor this later as in bug 731865
Created attachment 281610 [details] [review] Add PhotosGoogleItem
Review of attachment 281610 [details] [review]: Thanks for the updated patch, Saurav. ::: src/photos-google-item.c @@ +53,3 @@ + + +static GDataPicasaWebFile * It would be better to just return a GDataEntry. See below ... @@ +76,3 @@ + gdata_picasaweb_query_set_image_size (query, "d"); + + photo = gdata_service_query_single_entry (GDATA_SERVICE (service), authorization_domain, One parameter per line, please. @@ +84,3 @@ + g_object_unref (query); + + return GDATA_PICASAWEB_FILE (photo); In case of an error, 'photo' will be NULL and this cast will emit a WARNING. @@ +91,3 @@ +photos_google_item_create_thumbnail (PhotosBaseItem *item, GCancellable *cancellable, GError **error) +{ + GDataPicasaWebFile *photo = NULL; 'entry' will be a better name, because 'photo' is too similar to our project namespace (Photos, photos), and it matches with photos_google_get_picasaweb_file returning a GDataEntry. @@ +212,3 @@ + g_free (local_dir); + g_object_unref (local_file); + g_object_unref (remote_file); These should be g_clear_object too because they can be NULL when erroring out.
Created attachment 282584 [details] [review] Add PhotosGoogleItem I took the liberty to fix the above issues.
Thanks for all your work on this!
Thanks for the fixes and commit, Debarshi!