Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Rated sessions do not display as such when revisited. #1006

Open
tomconnell-wf opened this issue May 20, 2016 · 24 comments
Open

Rated sessions do not display as such when revisited. #1006

tomconnell-wf opened this issue May 20, 2016 · 24 comments
Labels

Comments

@tomconnell-wf
Copy link

I suspect that this starts happening when I get a new service worker.

Steps:

  1. Go to a session page.
  2. Rate the session.
  3. Notice that the Rate Session button is grayed out, and cannot be clicked. I believe the text changes in the button, also.
  4. Close that chrome tab.
  5. Return to that session in a new tab, the rate session button is clickable again.
  6. If you attempt to submit new feedback, you get a error message.

I did this on a Nexus 6p, mobile web.

@ebidel
Copy link
Contributor

ebidel commented May 20, 2016

We're aware of this issue. For some reason the session ratings aren't being stored in FB.

@pengying

@pengying
Copy link
Contributor

@crhym3 @nicolasgarnier

We debugged it and it seems like the backend isn't updating firebase.

@x1ddos
Copy link
Contributor

x1ddos commented May 21, 2016

The backend is updating firebase. otherwise, you wouldn't be able to complete steps 1-3.

@pengying
Copy link
Contributor

Only occurs in prod :/.
On May 20, 2016 11:17 PM, "alex" notifications@github.com wrote:

The backend is updating firebase. otherwise, you wouldn't be able to
complete steps 1-3.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1006 (comment)

@pengying
Copy link
Contributor

Sorry for being short at the after party. I meant it works fine in stage. But is only broken in prod.

@x1ddos
Copy link
Contributor

x1ddos commented May 21, 2016

So, in other words firebase security rules don't match. Is that what you're saying?

@ebidel
Copy link
Contributor

ebidel commented May 21, 2016

Alex we need help debugging. Can you take a look?

On Sat, May 21, 2016, 1:56 AM alex notifications@github.com wrote:

So, in other words firebase security rules don't match. Is that what
you're saying?


You are receiving this because you commented.

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

@x1ddos
Copy link
Contributor

x1ddos commented May 21, 2016

There's not much I see from the backend, just this:

401 Unauthorized
{"error" : "Permission denied"}

@pengying
Copy link
Contributor

@nicolasgarnier can you take a look at the permissions?

@nicolasgarnier
Copy link
Contributor

The permissions are the same on prod/dev/stage, I deployed the same rules automatically on stage/prod/dev typically and we deploy the same file.

You'll get a permission denied if the session has already marked as reviewed and you try to write to it though, as designed.

@crhym3 what were you trying to do when you got the error? Read or Write? Or did you just see this from the logs?
I'm guessing you are seeing this error from the logs, it must be coming from a tentative write on an already submitted session which is the intended behavior. That's how we check for already submitted sessions.

@nicolasgarnier
Copy link
Contributor

nicolasgarnier commented May 23, 2016

For now it seems that the data is indeed being written by the backend correctly. Peng you can check if the data is there by using the admin console for the user you are testing with:

https://io-2016-prod-<SHARD_NUM>.firebaseio.com/data/<UID>

If the data is there then this is likely just a UI issue.

@pengying
Copy link
Contributor

Can you share the Firebase with me or check
https://io-2016-prod-4.firebaseio.com/data/google:115132759088093329854

app.savedSurveys is always empty for me.
IOWA.Elements.Template.app.savedSurveys
[]

@ebidel
Copy link
Contributor

ebidel commented May 23, 2016

@pengying @nicolasgarnier , nothing is being saved:

screen shot 2016-05-23 at 4 50 57 pm

@nicolasgarnier
Copy link
Contributor

@pengying I shared all the prod DB with you

@nicolasgarnier
Copy link
Contributor

nicolasgarnier commented May 24, 2016

I think i found what the issue is:

https://io-2016-prod-9.firebaseio.com/data/google:115132759088093329854

image

Basically it seems the Backend is not writing to the same shard as the frontend. Something must be off in the Shard calculation the backend does. I could check the code if you point me to it.

Actually I found it: https://github.com/GoogleChrome/ioweb2016/blob/491d5f92629020b921739ea15a049606a8afacea/backend/survey.go#L67

@nicolasgarnier
Copy link
Contributor

nicolasgarnier commented May 24, 2016

I checked the Go backend code that calculates the shard and it seems correct:

https://play.golang.org/p/TQRUZAzvwP => returns 3 (which is good since that gives us shard io-2016-prod-4.firebaseio.com)

@pengying can you double check that the UID is passed correctly as a formValue to the backend? with no extra trailing/leading characters etc...? Maybe ideally could you print out the HTTP request done to the server?

@nicolasgarnier
Copy link
Contributor

Seems like the UID should always be passed correctly:
https://github.com/GoogleChrome/ioweb2016/blob/master/app/scripts/helper/schedule.js#L308-L312

@nicolasgarnier
Copy link
Contributor

nicolasgarnier commented May 24, 2016

I think I got it:

The issue is actually with the Go code and specifically at this line:

https://github.com/GoogleChrome/ioweb2016/blob/master/backend/survey.go#L68

gid := strings.TrimPrefix("google:", uid)

should instead be:

gid := strings.TrimPrefix(uid, "google:")

You can try this here: https://play.golang.org/p/DD4ZDSHuZO

gid := strings.TrimPrefix("google:", "google:115132759088093329854")
prints: google: instead of 115132759088093329854.

Basically right now all survey feedback are being written to shard 9 because that's what our sharding function returns for "google:"

@nicolasgarnier
Copy link
Contributor

nicolasgarnier commented May 24, 2016

We could fix it on the backend but I think it's basically a bit late to do that as we'd loose the data...
Instead we could also just have the client look at shard 9 always for all session feedback data since it's all there. I don't think the load is going to be an issue now that IO has passed.

@ebidel
Copy link
Contributor

ebidel commented May 24, 2016

Is the client always going to have access to shard 9?

On Tue, May 24, 2016, 6:16 AM Nicolas Garnier notifications@github.com
wrote:

we coudl fix it on the backend but I think it;s basically a bit late to do
that :D
Instead we could also just have the client look at shard 9 always for all
session feedback data since it;s all there. i don't think the load is going
to be an issue now that IO has passed.


You are receiving this because you commented.

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

@nicolasgarnier
Copy link
Contributor

yes we only restrict based on UID in the path which in this case match...

@ebidel
Copy link
Contributor

ebidel commented May 24, 2016

This isn't huge priority now that IO is over, but that seems like the most logical thing to do atm now that the data is all in that location :\

@pengying if you want to fix this, I'm supportive :)

@pengying
Copy link
Contributor

Haha. I mean I think we'll reuse the infrastructure for next year right? I think we should fix the backend and push out a version of the front end that looks at shard 9. But as part of the repo we should probably keep the non jacked up versions.

@ebidel
Copy link
Contributor

ebidel commented May 26, 2016

I like that plan

On Wed, May 25, 2016, 1:08 PM Peng Ying notifications@github.com wrote:

Haha. I mean I think we'll reuse the infrastructure for next year right? I
think we should fix the backend and push out a version of the front end
that looks at shard 9. But as part of the repo we should probably keep the
non jacked up versions.


You are receiving this because you commented.

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants