912696 - separate and user-surface saving from publishing

Status: RESOLVED FIXED

(Webmaker Graveyard :: Thimble, defect)

Description

[Pomax]

Right now we can only publish, which runs counter to the user idea of saving a project for private use and/or work, and publishing a project so that it's world-visible.

This ticket is here to help us track the things we need to do to separate saving from publishing.

Comment 1

[Chris DeCairos (:cade)]

So, this is kinda embarassing, but the MakeAPI has had an unused "published" field since v0.0.1 ( https://github.com/mozilla/MakeAPI/blob/ce5b18b99bff8bd65037867e98cac287faf1a93a/lib/models/make.js#L88 )

Could this be it's saving grace? only serve up "published" makes on the site, but show users their "unpublished" makes? there would really be no difference other than they could link things out privately.

Comment 2

[Chris DeCairos (:cade)]

(In reply to Chris DeCairos (:cade) from comment #1)
> there would really be no
> difference other than they could link things out privately.

clarification: If someone had the published link, they could view and share the make. it's private only in the sense that we choose to honour the field on webmaker.org. It could be made so that the query generator on webmaker adds in the filter (similar to deleted makes) so that published=false can't be circumvented with the search API.

Comment 3

[Chris DeCairos (:cade)]

(In reply to Chris DeCairos (:cade) from comment #1)
> Could this be it's saving grace?

its

Comment 4

[Pomax]

we can certainly jump on the "published" bandwagon and only improve things from there. Right now there is no "hidden" feature, not even "only half hidden", so anything we add that makes things a little more private (even if not fully) is a start. How much work would it be to change the MakeAPI so it filters out published=false, except for the authenticated user performing the search? (does our server-side makeapi querying include the initiating user's persona info?)

Comment 5

[Chris DeCairos (:cade)]

There's no persona related info passed along with search requests. Searches don't include authentication of any kind, so we can't trust persona related info associated with those requests.

Comment 7

[Pomax]

Attachment: https://github.com/mozilla/thimble.webmaker.org/pull/249

Comment 8

[Pomax]

this patch still misses S3-deleting of pages that have been marked as "unpublished", so that people can't still visit the page anyway because they happened to have had the link from some earlier sharing

Comment 9

[Jon Buckley [:jbuck]]

If the make doesn't appear in the MakeAPI, then the make-valet won't serve it up, so we're okay in that sense I think.

Comment 10

[Brett Gaylor [:brett]]

As discussed, moving from mozfest milestone

Comment 11

[Scott [:thecount] Downe]

So, probably not going to review this now, as we're not going to rush to push this for mozfest.

@jbuck:

make-valet is going to know how to serve it though, otherwise the creator of the project is never going to be able to preview it. Make-valet is going to have the ability to use the authenticated server side search. Either that, or make-valet and my makes need some login.

I don't see save/publish as private or public, that's a different thing, and if that's what we're trying to do, we shouldn't really call it save or publish, but instead have a private flag.

I see publish as going to webmaker.org. If not published it'll still exist in the details page, and in my makes, we just don't share it, we leave that up to the user to share it with who they decide to share it with. That way, they can still link it to people to get feedback.

I do like the idea of private, but not as much.

Comment 12

[Pomax]

@jbuck the make certainly goes into the makeapi, but with "plublished" set to false. It needs to, because otherwise you'd never see your (private) make in your list of "my makes", which would mean you saved it, and you can never access it ever again =)

Comment 13

[Jon Buckley [:jbuck]]

So just to clarify, everything will work exactly as it did before, but when you search for a users makes they will not appear in the result. It will work as a direct link though?

Comment 14

[Scott [:thecount] Downe]

@jbuck that's the idea. It's simply hidden.

Comment 15

[inactivate account for kate]

There was some discussion about changing the strategy around this, should I still review this?

Comment 16

[Scott [:thecount] Downe]

@kate I'm not reviewing it right now, or until requested, but I'm going to leave the review on me until then.

Comment 17

[Pomax]

contrary to thecount statement, I saw this feature as a proper privacy setting. Save, by default, should do a normal publish with the "published" flag set to "false" for the MakeAPI, and killing off the resultant S3 URL because thinking you're "not publishing" by virtue of not hitting the publish leading to a world-knowable URL is very strange, and a little creepy, and not very nice.

So save => leads to a make, and a loadable project for you to edit, but not to a working "content" URL or /remix url (the /edit url would still exist).

publish => leads to a normal make, content url, /edit url, /remix url.

Note that both options in this patch allow you to turn off actual publication through a checkbox, which tells Thimble what the user really wants. The save/publish buttons simply initiate it based on sensible preset for their perceived action. 

If you save an already-published project and you uncheck the checkbox, the project should update itself and tell the makeapi the "published" flag is now "false", and tell S3 to delete the content blob as well as the /remix blob. Checking the checkbox should tell the makeapi that "published" is true, and should effect an S3 republish for the content and /remix links.

Comment 18

[Pomax]

Comment on attachment 814677 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/249

removing review flags until post-MozFest

Comment 19

[Pomax]

Comment on attachment 814677 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/249

This patch works a treat, BUT: the published flag for makes is currently not respected in webmaker.org, so while this patch does need a review, we will not be able to land it until https://bugzilla.mozilla.org/show_bug.cgi?id=919710 is resolved.

Comment 21

[Scott [:thecount] Downe]

Comment on attachment 814677 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/249

It does work well.

Some UI/UX ideas nits though.

The ellipsis on long title names is now covered by the save button.

I wonder how hard it would be to grey out the buttons as they are used. Example, if I click save, grey out the save button until I make another change to the project, but publish is still clickable. If I click publish, grey out both, and if another change is made make them both clickable again. Totally think this can be a new feature in a new ticket, once this all lands if you like though.

I like the "Treat this make as published, rather than merely saved" option in the save/publish dialog, but I wonder if we could make the text in the save/publish button toggle as that checkbox is toggled, seeing this visual update confirms with people what just happened. Also, the checkbox is not aligned with the text.

Also one single whitespace nit.

Comment 22

[Pomax]

good suggestions on button states and labels, I think we might as well work that into this ticket rather than as a followup, as long as 919710 is still unresolved.

Comment 23

[Pomax]

Attachment: https://github.com/mozilla/thimble.webmaker.org/pull/341

I'll file a followup ticket for the improved UX when saving/publishing/typing new content.

Comment 24

[Scott [:thecount] Downe]

Comment on attachment 8359998 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/341

Looks good.

A new ticket for toggling state of the buttons is fine with me. I feel like toggling the state gives the user some sort of feedback as to what has happened and what state something is in, but it's not critical.

Once the blocking ticket lands, it would make sense to give this patch a sanity check.

Comment 25

[Pomax]

Attachment: https://github.com/mozilla/thimble.webmaker.org/pull/350

Comment 26

[Pomax]

Comment on attachment 8368860 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/350

Rebase to today's master, as far as I can tell this does the right thing but I need a sanity change because the rebase was painful.

Comment 27

[Jon Buckley [:jbuck]]

Comment on attachment 8368860 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/350

If I save a make using the save button, then attempt to publish that make using the publish button it prompts me to change the make name: http://dl.dropbox.com/u/4403845/Screenshots/dl.png

Comment 28

[Scott [:thecount] Downe]

I'm not getting the name change error. Everything seems to work as expected.

Comment 29

[Pomax]

Comment on attachment 8368860 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/350

Looking at the network behaviour, there's some interesting magic that needs to be fixed:

1. save => publish route called
2. publish with same title => publish route called. twice. One succeeds, one fails.
3. publish again with same title => publish route called, three times. again one fails.
4.  ""  => publish route called one additional time, etc. etc.

I suspect this is where the failings are coming from, and will investigate the code and fix, then reset review requests.

Comment 30

[Pomax]

Comment on attachment 8368860 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/350

patch fixed, the publish call now only goes out once when "publish" or "save" is clicked in the actual save/publish dialog. This should fix the error that jbuck saw, but please confirm this.

Comment 31

[Jon Buckley [:jbuck]]

Comment on attachment 8368860 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/350

I can recreate my multiple-publish bug by:

1) Open a new page
2) Click the Save or Publish toolbar button, then close it by clicking outside the dialog box
3) Click the Save or Publish toolbar button, then try to Save or Publish inside the dialog
4) It will fail to Save or Publish, and have a number of failing XHRs in the Web Console

More events being queued up somehow?

--

I also have a question about the implementation. In Popcorn, we're going with a single Save button that has a "Published on webmaker.org" checkbox. I think it'd make sense to do the same thing here, to keep the save/publish UX consistent between these two tools. I also think it should be "Published on webmaker.org" by default. Thoughts?

Comment 32

[Pomax]

Comment on attachment 8368860 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/350

updated to simply clean up click events before binding now, this should properly fix things.

Comment 33

[Jon Buckley [:jbuck]]

Comment on attachment 8368860 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/350

Can't break it this time!

Comment 34

[[github robot]]

Commit pushed to master at https://github.com/mozilla/thimble.webmaker.org

https://github.com/mozilla/thimble.webmaker.org/commit/800be65fe7905095cadc06edfc76736c775dde7a
Merge pull request #350 from Pomax/bug912696v3

Separated save from publish, hooking into the makeapi's "published" flag