Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Public Draft Preview via uuid #5158

Closed
wants to merge 1 commit into from
Closed

Conversation

novaugust
Copy link
Contributor

Add post preview via uuid (/p/:uuid)
Refs #5097

  • All drafts will show a preview link (this needs real css)
  • Published posts will redirect to their frontend url

Original writeup: It says so in the title, but this is just proof of concept i coded up while on a plane in the hopes of starting a movement towards getting previews implemented. I've gotten tired of using the "temporarily publish as page" trick ;)

@@ -285,6 +285,46 @@ frontendControllers = {
}).catch(handleError(next));
},

preview: function (req, res, next) {

This comment was marked as abuse.

This comment was marked as abuse.

@ErisDS
Copy link
Member

ErisDS commented Apr 20, 2015

Yo @novaugust - just so I know, are you looking for some feedback or collab? Can do either 😁

@novaugust
Copy link
Contributor Author

Not even sure! :D Feedback'd be great. I just wanted to throw this code up somewhere before I disappeared travelling :P I mean, it doesn't really fulfill the specs of the "preview" issue anymore, but it does go a good ways to enabling nice functionality .. I put it up a bit scatter-brained with no real intention other than seeing what people thought, I suppose.
Honestly, I'm fine with just closing the PR and moving on, though it could be useful just as a way to point new contribs of where to play for this issue.

@ErisDS
Copy link
Member

ErisDS commented Apr 21, 2015

One thing to note, if we switch to using the hashids module as per the v2 spec then we can do a lookup based on the post's id, instead of needing to change the API to look up the uuid, so that makes things easier.

The key other part here is making sure the preview URL is private for the time being. The public sharing part of this can be added later.

@novaugust
Copy link
Contributor Author

That's an easy addition to this, I'd say. Might not be able to get at it
for a week; maybe I'll just hack it out with you in person ;)

On Tue, Apr 21, 2015 at 6:20 PM, Hannah Wolfe notifications@github.com
wrote:

One thing to note, if we switch to using the hashids module as per the v2
spec #5097 then we can do a
lookup based on the post's id, instead of needing to change the API to look
up the uuid, so that makes things easier.

The key other part here is making sure the preview URL is private for the
time being. The public sharing part of this can be added later.


Reply to this email directly or view it on GitHub
#5158 (comment).

@ErisDS
Copy link
Member

ErisDS commented Apr 21, 2015

Let's do that :)

@ErisDS
Copy link
Member

ErisDS commented Apr 28, 2015

I know you've written a lovely big comment over on the issue, but just wanted to clarify what's going on here.

After a bit of a hack session looking at doing this, we effectively reverted back to the original spec as a first iteration. This version of previews uses the unguessable 36 char UUID to create URLs which are secret but not enforced private.

Moving to private previews, adding per-post sharing options, and shorter URLS are all iterations on this feature which are significantly complex enough to warrant doing separately.

This PR will need a little bit of a followup commit from @JohnONolan to fix the CSS for displaying the preview URL (unless you want to tell us how to fix it 😁), but otherwise is ready to go.

I don't think it needs to go behind a labs flag.

@novaugust novaugust force-pushed the preview branch 2 times, most recently from cf77b30 to 02dc189 Compare April 28, 2015 12:36
@novaugust novaugust changed the title [wip] Preview Public Draft Preview via uuid Apr 28, 2015
}).catch(function (err) {
// If we've thrown an error message
// of type: 'NotFound' then we found
// no path match.

This comment was marked as abuse.

@novaugust novaugust force-pushed the preview branch 5 times, most recently from 76e9a85 to 134e289 Compare April 30, 2015 09:18
Refs TryGhost#5097

- All drafts will show a preview link (this needs real css)
- Published posts will redirect
- Powered by ~10 pints between the two of us (@ErisDS, @novaugust)
@novaugust novaugust closed this Apr 30, 2015
@novaugust novaugust deleted the preview branch April 30, 2015 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants