Group-income-simple: Improve router usage after a group is created.

Created on 3 Oct 2019  路  5Comments  路  Source: okTurtles/group-income-simple

Problem

Context: https://github.com/okTurtles/group-income-simple/pull/679/files#r329122719

There was a weird rare false positive in a test failure in this step. Let me explain:

image

This view is visible for 1ms. It happens immediately after a user with a group logs in. How is that possible? Well...
Let's see the current conditions for each view:

  • sidebar: isloggedIn and hasGroups
  • welcomeGroup: isloggedIn

So, both conditions are indeed true.

If we look at the code, after the login happens, there's a tiny moment where we verify it the user has a group and then ask $router to redirect to /dashboard.

// Login.vue#L91
await sbp('state/vuex/dispatch', 'login', { username, identityContractID })
if (this.$store.state.currentGroupId) {
  this.$router.push({ path: '/dashboard' })
}

That's the exactly moment where this interface (the screenshot) takes place in time.

For that reason, the condition to find "welcomeHomeLoggedIn" (L33) is positive. However, in the next millisecond, when we try to get the element again and assert the text, the element isn't there anymore because the $router has changed to /dashboard.

Based on this, it's a tricky task to get the correct element on the page in this moment. So I decided to change the assertions: if the login was successful, it means we won't display the login button anymore. That's it.

I think the ideal solution would be to move the welcome view (create/join group) to another router (ex: /welcome). That would prevent this bug and we would still have better login assertions.

Solution

Improve how we are using the router /welcome and how we present that to the users after a group is created.

cc @pieer and @taoeffect

Frontend Bug Test

Most helpful comment

Probably going to tackle that next week @taoeffect but I'm not sure that creating the new router is the solution as per @mmbotelho request the welcome part is at the end of the popup group creation.

All 5 comments

@sandrina-p I think you have the best understanding of this issue out of everyone here so if it's alright I've tagged and assigned this issue to you. If you want to leave it as Up-for-Grabs feel free to unassign and re-tag the issue!

Probably going to tackle that next week @taoeffect but I'm not sure that creating the new router is the solution as per @mmbotelho request the welcome part is at the end of the popup group creation.

@pieer ok, then I've assigned it to you!

@taoeffect as to the '/welcome' route you mentioned here, it's actually not being used anymore and can be removed(I was the one who created it).
the group creation process is currently implemented through modal, so the url for 'the welcome confetti animation page' would be something like http://localhost:8000/app/?modal=CreateGroup&step=GroupWelcome right now. (it used to be, http://localhost:8000/app/welcome).

Do you think I need to make a pull request for removing it before someone starts working on this issue?

c.c. @sandrina-p

Copying my answer from Slack here:

I think if it鈥檚 no longer being used then there鈥檚 no harm in submitting such a PR if you鈥檇 like!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

taoeffect picture taoeffect  路  8Comments

sandrina-p picture sandrina-p  路  8Comments

sandrina-p picture sandrina-p  路  6Comments

hubudibu picture hubudibu  路  5Comments

taoeffect picture taoeffect  路  3Comments