Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

post-connect lands on 404 #421

Closed
chadwhitacre opened this issue Dec 17, 2012 · 11 comments
Closed

post-connect lands on 404 #421

chadwhitacre opened this issue Dec 17, 2012 · 11 comments

Comments

@chadwhitacre
Copy link
Contributor

Reticketing from #313.

@chadwhitacre
Copy link
Contributor Author

Okay, working backwards:

Where does the redirect come from?

  • /on/take-over.html -> /about/me.html?
  • /on/twitter/associate -> /about/me.html?
  • /on/twitter/associate -> /on/twitter/%login/?
  • where else?

@chadwhitacre
Copy link
Contributor Author

@sigmavirus24 hit this as well, and in his case he was connecting for the first time, not merging. My understanding is that @davisagli was merging.

So for the first time case, it should have created a stub participant for the new Twitter account and then performed the merge without confirmation. @sigmavirus24 confirms that the merge happened transparently. The experience was coming back from Twitter to a 404.

@chadwhitacre
Copy link
Contributor Author

@sigmavirus24:

  • sign in using GitHub
  • click "Twitter account"
  • go through Twitter oauth

@davisagli:

  • sign in using GitHub
  • click "Twitter account"
  • ...

I bet @davisagli also had never connected a Twitter account before. For both @sigmavirus24 and @davisagli, the value of archived_as is also coming up 404, whereas for the remainder ...

@chadwhitacre
Copy link
Contributor Author

Yes, I checked: the remainder of the people who have merged accounts so far are not 404 for archived_as. That gives me the clue to repro.

@chadwhitacre
Copy link
Contributor Author

Okay, reproduced locally ...

@chadwhitacre
Copy link
Contributor Author

The redirect doesn't come through take-over.html, it comes through associate.

@chadwhitacre
Copy link
Contributor Author

Got it! ...

chadwhitacre added a commit that referenced this issue Dec 17, 2012
We weren't modifying account appropriately when modifying user.
Presumably this is the sort of thing SQLAlchemy is designed to protect
against.
@chadwhitacre
Copy link
Contributor Author

Well, one of them. The actual bug that started this ticket is still present, but I'm able to reproduce it now that the other bug is fixed.

@davisagli
Copy link

Yes I was also attaching a twitter account for the first time rather than merging.

@chadwhitacre
Copy link
Contributor Author

Awesome, thanks for confirming, @davisagli. Fix on its way ...

@sigmavirus24
Copy link
Contributor

✨ 🤘 ✨ 🍰 ✨

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

No branches or pull requests

3 participants