rabblerouser / core

Pluggable, extensible membership database for community organising
GNU Affero General Public License v3.0
17 stars 10 forks source link

Error message after saving branch [racing condition problem] #157

Open pameck opened 7 years ago

pameck commented 7 years ago

Error Description:

After a branch is created, the following message is displayed:

screen shot 2017-04-12 at 9 15 13 am

In the dev console, there is this:

screen shot 2017-04-12 at 9 15 41 am

To replicate:

  1. Go to the admin dashboard
  2. Create a new branch
  3. See error.

Possible cause:

When a new branch is created, there are two subsequent requests:

GET /branches/{id}/groups GET /branches/{id}/members

The problem is that we're adding the new branch to the core's store after it has gone through the event pipe, lambdas and back to core, but the requests from the UI get to the core before the new branch is added to the store which is why those two requests get a 404 error.

The following diagram is for reference about the branch creation flow:

branches-with-events 1

yearofthedan commented 7 years ago

It's an interesting problem. A couple of options come to mind.

camjackson commented 7 years ago

Wait, so, why is the frontend even making those requests? I don't think it's an ID generation issue, because the backend generates the ID before putting the event on the stream, and it will return that ID to the frontend synchronously.

From @pameck's screenshot, it seems the frontend is trying to fetch the new branch's groups and members, which is probably unnecessary. I think it's safe to assume that a brand new branch doesn't have any groups or members yet, so those fields can just be initialised to empty arrays, rather than fetching empty arrays from the backend.

So proposed solution: just don't make those requests 😁

yearofthedan commented 7 years ago

I havent looked too deep, but I think the issue is that the app selects the newly created branch which triggers a new API call for the branch details. So that would mean either not selecting, or creating some new flow.

pameck commented 7 years ago

I think that on top of the change in the UI, the API should return an empty array instead of a 404 when asking for a collection of things.

yearofthedan commented 7 years ago

But also you're right; ignore what I said about IDs. I had a blip and forgot that the create request returns one.

yearofthedan commented 7 years ago

Even if the branch can't be found?

pameck commented 7 years ago

@yearofthedan right! It's the branch that's missing not the groups or members, back to the 404 😬