search

code-dot-org / dance-party

Renderer for the Dance game type. Based on p5.js and p5.play.js.
https://code-dot-org.github.io/dance-party/
13 stars 13 forks source link

We aren't showing all published dance party projects in the gallery #402

Closed poorvasingal closed 5 years ago

poorvasingal commented 6 years ago

Data shows there are over 800 published projects

SQL query: select count(*) from storage_apps where project_type = 'dance' and published_at is not null

But I'm only seeing about 30 projects in the gallery when I'm trying to see all dance party projects.

Maybe this is based on where we're publishing projects from? (e.g. standalone vs. from a level).

poorvasingal commented 6 years ago

CC @Erin007

Erin007 commented 6 years ago

Erin's investigation notes (no need for anyone else to read unless they're curious, just keeping myself organized)

Erin007 commented 5 years ago

aha!

Good news: I think I finally know the root of the problem.
Bad news: The problem is not specific to DanceParty.

I was so focused on a potential mismatch with the project data and what was showing up in the gallery that I missed this line https://github.com/code-dot-org/code-dot-org/blob/7ec46cfabf98643c30958bb7ae9d1959bcdee188/dashboard/lib/projects_list.rb#L231 which says to only pull projects published by users for whom sharing is enabled. If you're under 13 sharing is disabled, unless you are in a section for which the teacher has enabled sharing. I think the real underlying issue here is that we let students "publish" projects, that is we let them click on the share button, open the share dialog and mark projects as published in the database, but the gallery still won't show them because sharing is disabled for the given user. I can confirm by signing in as a young student, making a Dance Party project, going through the steps to publish the project, then the project unexpectedly does not show in the gallery even though the database has the project with the correct info and the published_at field set to the current date.

This is currently the case for all project types; we just noticed it for Dance Party because that project type is brand new so the gallery is slowly filling and we don't have many featured Dance Party projects to mask the issue yet. @boatmarina (or @poorvasingal, but I know you're out) can you confirm that the fix here is to restrict showing of App Lab/Game Lab projects in the public gallery if sharing is disabled, but all other project types should show in the gallery when published regardless of whether the user has sharing disabled? Maybe @islemaster or @davidsbailey have thoughts too?

davidsbailey commented 5 years ago

Nice job getting to the bottom of this, @Erin007 ! What you are proposing sounds right on.

islemaster commented 5 years ago

Agreed, we should allow publishing of most project types (dance, artist, etc) for students of any age. There are some PII concerns, but that's why we have the PII+Webpurify filtering on Play Lab projects.

Erin007 commented 5 years ago

Fix: https://github.com/code-dot-org/code-dot-org/pull/26173

ghost commented 5 years ago

Correct. All project types are publishable regardless of age except for AppLab, gamelab and weblab (and these are allowable for under 13 if in a section and the teacher has permitted it.) the right fix would be to hide the actual publish button in these cases.

But we can fix this in whatever way is simplest for HOC - important result should be that all dance party projects show up and IF a teacher has not allowed sharing for the “advanced” project types that these don’t show up (default for sections with students under 13 is that they can’t share.) Note, a student that isn’t in a section can only access AppLab and gamelab if they are above 13 so if they’re not in a section, they should be able to publish.

On Mon, Nov 19, 2018 at 10:09 PM Erin Bond notifications@github.com wrote:

Fix: code-dot-org/code-dot-org#26173 https://github.com/code-dot-org/code-dot-org/pull/26173

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/code-dot-org/dance-party/issues/402#issuecomment-440155933, or mute the thread https://github.com/notifications/unsubscribe-auth/AgSF29wt40e3F401J4oFyXLh0Uo7v_Htks5uw5yHgaJpZM4Yhjz3 .

poorvasingal commented 5 years ago

Confirming that we actually already do hide the "Publish" button for App Lab and Game Lab if you're under 13: image

So I think we should be good there.

ghost commented 5 years ago

What about in the finish/share dialogs?

On Tue, Nov 20, 2018 at 8:32 AM poorvasingal notifications@github.com wrote:

Confirming that we actually already do hide the "Publish" button for App Lab and Game Lab if you're under 13: [image: image] https://user-images.githubusercontent.com/16227893/48788015-eb8f2f80-ecb7-11e8-9cf4-1989168f82b8.png

So I think we should be good there.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/code-dot-org/dance-party/issues/402#issuecomment-440338038, or mute the thread https://github.com/notifications/unsubscribe-auth/AgSF2yQMkc4GjY3E9yZH3wPMQxBeSsTyks5uxC6ggaJpZM4Yhjz3 .

poorvasingal commented 5 years ago

@boatmarina - Glad you made me just test this! App Lab is behaving as expected in the places I checked:

Share: image

Finish: image

But apparently the Finish dialog for some levels is still broken: image

I think this broke because of changes we were trying to make to CSF. We're never supposed to show the share link in the finish dialog of App Lab / Game Lab levels (regardless of age). CC @ryansloan to track for later.

This isn't urgent to fix b/c students still aren't seeing the publish button and if you try to go to the share link as someone other than the student or teacher, you can't access the project.

ghost commented 5 years ago

ok great, thanks for testing. so sounds like the only change we need from Erin is to ensure that DanceParty projects show up regardless of age, right?

On Tue, Nov 20, 2018 at 8:44 AM poorvasingal notifications@github.com wrote:

@boatmarina https://github.com/boatmarina - Glad you made me just test this! App Lab is behaving as expected in the places I checked:

Share: [image: image] https://user-images.githubusercontent.com/16227893/48788460-de267500-ecb8-11e8-85dd-b42a9a637c42.png

Finish: [image: image] https://user-images.githubusercontent.com/16227893/48788533-07470580-ecb9-11e8-925c-92bf758c10e5.png

But apparently the Finish dialog for some levels is still broken: [image: image] https://user-images.githubusercontent.com/16227893/48788714-64db5200-ecb9-11e8-9d9a-9270af6dfd4f.png

I think this broke because of changes we were trying to make to CSF. We're never supposed to show the share link in the finish dialog of App Lab / Game Lab levels (regardless of age). CC @ryansloan https://github.com/ryansloan to track for later.

This isn't urgent to fix b/c students still aren't seeing the publish button and if you try to go to the share link as someone other than the student or teacher, you can't access the project.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/code-dot-org/dance-party/issues/402#issuecomment-440342561, or mute the thread https://github.com/notifications/unsubscribe-auth/AgSF28MYUe1B7b-AsrxWT7DLBAStjGRNks5uxDFugaJpZM4Yhjz3 .

Erin007 commented 5 years ago

" the only change we need from Erin is to ensure that DanceParty projects show up regardless of age, right?" sounds like the correct outcome to me. The fix I prepped in the PR above will show all project types, including Dance Party, except App Lab, Game Lab and Sprite Lab (@islemaster had some concerns about PII since we're not checking those like we do Play Lab yet) regardless of the owner's sharing restrictions. Thanks for the input, everyone!

Erin007 commented 5 years ago

Dance Party project gallery is gloriously full now!