930039 - Saved Popcorn projects load a blank project by mistake if you don't have session set with popcorn.wm.o

Status: VERIFIED FIXED

(Webmaker Graveyard :: Popcorn Maker, defect)

Description

[Chris DeCairos (:cade)]

STR:

1. ensure you are logged out of popcorn.webmaker.org and close all tabs 
2. log in on webmaker.org and open your makes page (  /me )
3. click the edit button on a popcorn make

Expected: you are automatically signed into pocporn, and your saved project is loaded.

Actual: you are shown a remix of your project, and are signed in.

Comment 1

[Matthew Schranz [:mjschranz]]

This is an issue with our SSO that to me knowledge we sadly are not able to overcome right now.

Comment 2

[Gervase Markham [:gerv]]

:mjschranz: are you able to elaborate?

I find it hard to figure out why it's not possible to make it that clicking a URL which ends in /edit either ends up in an editing session or a login box, or at worst some sort of error, and does not end up in a /remix URL. The current behaviour is deeply confusing for students, who will end up with multiple copies of the same project. If they then end  up back at the original one, it'll seem like they've lost a load of work.

Gerv

Comment 3

[Scott [:thecount] Downe]

Yeah, that's not obvious at all.

Needs a solution.

Comment 4

[Scott [:thecount] Downe]

Right now going to /edit while not logged in gives me a blank project. It should load the project even if I am not logged in. Then when I go to save, it can figure out if I own the project or not (I have to be logged in at this point) and then it can decide to save or remix the project, depending on if I own it or not.

Comment 5

[Scott [:thecount] Downe]

Reopening this because I think this might be causing some of the confusion of "lost projects" people have been experiencing.

Comment 6

[Scott [:thecount] Downe]

Attachment: https://github.com/mozilla/popcorn.webmaker.org/pull/369

What I am attempting to do is keep the project in remix or edit state not just via page refresh, but log in/log out.

Examples of things this can now do:

1. If I am in /edit, and I am not signed in, it starts me off as a remix, instead of an empty project.
2. As soon as I sign in, it checks if I own the page, if I do, it changes from remix to edit, if the url is /edit.
3. If I do not own the page, and I log in as someone else, it keep it as a remix.

Thoughts?

I feel like this could share a lot more code, but I wasn't able to refactor it all yet. There is a lot going on here.

Comment 7

[Jon Buckley [:jbuck]]

Comment on attachment 8343620 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/369

So, we could handle this through the editor but it feels really weird to do it through the editor.

What if we modified the server to send back the right data, with or without a valid login?

If the SSO kicks in properly then we don't even need to worry about handling the remix vs edit use case. When user x attempts to use user y's project, it'll just save a copy into user x's profile.

Comment 8

[Scott [:thecount] Downe]

I started by making the /edit path simply not need a login, but then I found the old path that was only really blocked because of a bug. I was also adding a bunch of new bugs, so I was happy to re enable it. :P

See this line: https://github.com/mozilla/popcorn.webmaker.org/pull/369/files#diff-bc81a5fca7f6933ee14be8ca2ce3e068R1054

This change alone is what allows a user to hit up an edit path they do not own, and get a remix out of it.

Is it worth changing how we do this? I feel safer re enabling a code path we can somewhat trust, and leaving existing code we can trust a lot the way it is. I'm more than willing to give it another shot, but it does worry me and I want to be sure we think it is worth it.

I do however want to find the reason as to why that data went from json to a string, if that was expected, and should stay a string, then we want something along the lines of my fix. If we could make the data be json again, and have that be secure, that would also work.

The rest was just listening to login and out, and sending the appropriate data off to the project module. Is that end of the patch something you're OK with? I wonder how it would work on it's own, and if we should split up this ticket into two?

Anyway, I'll bug you tomorrow about this.

Comment 9

[Scott [:thecount] Downe]

Comment on attachment 8343620 [details] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/369

Updated.

Going to file a ticket on the UI updates once this lands.

Comment 10

[Scott [:thecount] Downe]

Staged: https://github.com/mozilla/popcorn.webmaker.org/commit/815ad68f2502f9bef8c866cd52b97a953688972b

Needs verification.