port Piwigo publishing to new API
Submitted by Adam Dingle
Link to original bug (#717491)
Description
---- Reported by adam@yorba.org 2011-02-15 15:30:00 -0800 ----
Original Redmine bug id: 3203
Original URL: http://redmine.yorba.org/issues/3203
Searchable id: yorba-bug-3203
Original author: Adam Dingle
Original description:
port Piwigo publishing to new API
---- Additional Comments From shotwell-maint@gnome.bugs 2013-05-01 11:39:00 -0700 ----
History
Comment 1
Updated by Adam Dingle almost 3 years ago
- Status changed from Open to Review
- Assignee changed from Anonymous to guillaumev -
-
Target version deleted (
<strike>
_0.9_</strike>
)
Comment 2
Updated by Adam Dingle over 2 years ago
- Target version set to 0.10
Comment 3
Updated by Adam Dingle over 2 years ago
-
Target version deleted (
<strike>
_0.10_</strike>
)
Comment 4
Updated by Adam Dingle over 2 years ago
- Assignee changed from guillaumev - to Anonymous
Comment 5
Updated by Bruno Girin over 2 years ago
- Assignee changed from Anonymous to Bruno Girin
Comment 6
Updated by Bruno Girin over 2 years ago
Here's a first patch that implements the bulk of the plugin. Unfortunately, it seems that Piwigo gets confused by the file name in the upload request and doesn't really upload the file (even though it claims to have done so) so any suggestion as to how to solve this would be welcome.
Other known bugs:
- The dialog panes were created using Glade in an attempt to simplify the Vala code but they force the dialog to expand. There are probably some packing options missing in the code somewhere.
- The publishing options pane needs some polishing: it needs to ensure that the Publish button is only enabled when the input makes sense and it would be nice to persist and retrieve previous selections to present them back to the user next time round.
- Other potential improvements are marked as TODO comments in the code.
I have a test Piwigo server that you are welcome to use here: http://ec2-46-137-27-112.eu-west-1.compute.amazonaws.com/piwigo/ user name is admin, password is piwigo. The admin user can also create additional test users if required.
Comment 7
Updated by Adam Dingle over 2 years ago
Thanks a lot for your help on this, Bruno. We'll look at this and give you some feedback soon.
Comment 8
Updated by Adam Dingle over 2 years ago
- Target version set to 0.10
Comment 9
Updated by Adam Dingle over 2 years ago
Bruno: By the way, will this Piwigo port introduce new strings to be translated? We'd like to freeze strings for 0.10 this week if possible.
Comment 10
Updated by Bruno Girin over 2 years ago
Replying to [comment:10 adam]:
Bruno: By the way, will this Piwigo port introduce new strings to be translated? We'd like to freeze strings for 0.10 this week if possible.
Yes it will introduce new strings as I've slightly modified some of the UI labels and added a couple of error messages. However, the current patch already includes all new translatable strings and further fixes should not affect them.
Comment 11
Updated by Bruno Girin over 2 years ago
Additional patch attached. This corrects a compiling error in the previous patch (sorry!) and adds support for message request headers in UploadTransaction in order to support the Piwigo cookie. There is still a problem with the upload though so I'll have to look into this.
Comment 12
Updated by Lucas Beeler over 2 years ago
Replying to [comment:12 brunogirin]:
Additional patch attached…
Hi Bruno,
First of all, thank you so much for all of your hard work on this! You're a fantastic Shotwell contributor and I hope we get to meet up again at GUADEC this year!
Overall, the patch seems to be coming along well. A few things:
The reason why the dialog expands isn't because of your glade file. Instead, it's because when the authentication pane is queried for its geometry options, you return the Spit.Publishing.DialogPane.GeometryOptions.EXTENDED_SIZE flag. This isn't necessary, since all the text you have will easily fit in a standard-size pane and it's distracting for the user to see the dialog change sizes. To get the standard pane size, just return Spit.Publishing.DialogPane.GeometryOptions.%(=caps)NONE% in your get_preferred_geometry( ) method instead.
Make sure you return Spit.Publishing.DialogPane.GeometryOptions.%(=caps)NONE% in response to the get_preferred_geometry( ) geometry query on the Publishing Options Pane too.
In the Publishing Options Pane, if the user has opted to publish to an existing category, then some category should be selected in the combo box that lists the categories. Ideally, it should be the last one the user selected, but it'd be fine if we just selected the first one as an interim step. The problem now is that when the combo box is first displayed, there's nothing selected so it appears blank.
Once again, thanks for all the hard work!
Cheers,
Lucas
Comment 13
Updated by Bruno Girin over 2 years ago
Lucas, no idea why the code returns the Spit.Publishing.!DialogPane.!GeometryOptions.EXTENDED_SIZE in the geometry options. I must have copied some code from some other class at some point and left it there. I will change it to NONE on both panes.
As for the Publishing Options Pane, totally agree, it needs to offer a default selection to the user, ideally the last one selected otherwise a sensible default. I'll update that once I've worked out why it doesn't publish properly yet.
Comment 14
Updated by Bruno Girin over 2 years ago
See four different patches added. This cleans up the way the dialogs behave and fixes the management of the cookie. There is one last piece missing: the final publishing call to the addSimple web method doesn't work as it looks like the Piwigo server doesn't actually save the image on disk properly. It's probably a request parameter not being set properly but looking at the HTTP call in Wireshark I can't see what's missing.
Comment 15
Updated by Bruno Girin over 2 years ago
Additional patch attached. Pierrick had a quick look at the HTTP output and found the obvious mistake so the plugin now fully works. It needs a few usability improvements which I will add in the next few days.
Comment 16
Updated by Adam Dingle over 2 years ago
Bruno, you're a hero! Thanks again for all your hard work on this. Lucas, please review!
Comment 17
Updated by Bruno Girin over 2 years ago
Added an additional patch to handle field changes in the publishing pane so that you can only click “Publish†with a sensible combination of fields.
Comment 18
Updated by Bruno Girin over 2 years ago
A couple more patches to make the publishing pane remembers its settings and to strip a new category name of leading and trailing spaces.
Comment 19
Updated by Lucas Beeler over 2 years ago
Bruno, could you post a composite patch that has all of your changes in one diff? That'd make it much easier to review than applying differential patches on top of each other!
Comment 20
Updated by Bruno Girin over 2 years ago
Lucas, I think I can do that with git diff rather than git format-patch so will do when I get home tonight and will post a composite patch. That may not be before 9 or 10pm GMT though so I hope it won't be too late for you to review.
Comment 21
Updated by Bruno Girin over 2 years ago
Jumbo patch attached.
Comment 22
Updated by Lucas Beeler over 2 years ago
Bruno,
Awesome work! Some feedback on your patch:
Right now, in the login pane, the “Close†button has default action focus. This should be fixed so that the “Login†button has the default action focus. Most users expect that when they fill out their login details and then press [%(=caps)ENTER%] that the action taken will be to login to the service. But that's not what happens today. Right now, when the user presses [%(=caps)ENTER%] in the login pane, the publishing dialog is dismissed and publishing is canceled. Note that the PluginHost interface has a method for this exact purpose: set_dialog_default_widget( ). Since the “Login†button will become the default button, it should remain insensitive until the user has filled out all of their login information.
I had intermittent problems logging on to the Piwigo server you set up, but that might well be on amazon's end and have nothing do to with your plug-in.
Because you did a git diff to produce a composite diff (which was great! thank you so much!) binary files were not included, so could you please attach the PNG icon you use for the Piwigo publisher to this ticket since it wasn't included in the diff?
The name you're currently using for your PublishingService is “Piwigo (new).†You can now change this to simply “Piwigo†since we've ripped all the old publishing code out. You're Mr. Piwigo from now on!
As expected, Bruno, your code itself is excellent. Thank you so much for your help!
Comment 23
Updated by Bruno Girin over 2 years ago
PNG icon attached as well as an additional patch to correct default button handling on both dialog panes, only enable the login button when everything is filled in and remove the (new) string.
The reason why you can't login to the Piwigo server I setup is because I stopped it at the week-end as I wasn't sure I had installed it correctly. I used a test account on piwigo.com instead. Sorry, I should have mentioned that.
Comment 24
Updated by Lucas Beeler over 2 years ago
- Resolution set to fixed
- % Done changed from 0 to 100
Committed in 6b8e5dae. Two-hundred-thousand thanks to Bruno!
Comment 25
Updated by Vera Yin over 2 years ago
Lucas, should this be committed to the 0.10 branch?
Comment 26
Updated by Lucas Beeler over 2 years ago
Indeed it should be. Too much cold medicine. I'll git cherry-pick it over to the branch now. Thank you so much for noticing! You're a lifesaver!
Comment 27
Updated by Lucas Beeler over 2 years ago
Done.
Comment 28
Updated by Charles Lindsay 7 months ago
- Status changed from 5 to Fixed
--- Bug imported by chaz@yorba.org 2013-11-25 21:51 UTC ---
This bug was previously known as bug 3203 at http://redmine.yorba.org/show_bug.cgi?id=3203 Imported an attachment (id=261995) Imported an attachment (id=261996) Imported an attachment (id=261997) Imported an attachment (id=261998) Imported an attachment (id=261999) Imported an attachment (id=262000) Imported an attachment (id=262001) Imported an attachment (id=262002) Imported an attachment (id=262003) Imported an attachment (id=262004) Imported an attachment (id=262005) Imported an attachment (id=262006) Imported an attachment (id=262007)
Unknown Component Using default product and component set in Parameters Unknown milestone "unknown in product shotwell. Setting to default milestone for this product, "---". Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one.
Version: 0.10
Resolution: RESOLVED FIXED