Uncomment role checks in dbAuth's auth.js template

[cannikin]

Not sure why I would have commented these out, I thought I just copied the standard auth.js template and make a couple of changes for getCurrentUser() unique to dbAuth.

[thedavidprice]

@dthyresson did you mean to comment these when you were resolving the TS errors over here 20cb526?

[dthyresson]

I don’t think so, they should be there. But will check for typing.

[dthyresson]

Oh, I see:

For DbAuth, since the User model doesn't yet have roles, the role check is commented out and the developer is informed to re-enabled them as needed

Maybe with the new Tutorial examples, roles will exist? @cannikin

Premature approval

[dthyresson]

For DbAuth, since the User model doesn't yet have roles, the role check is commented out and the developer is informed to re-enabled them as needed

Maybe with the new Tutorial examples, roles will exist? @cannikin

That's why they were commented out.

[cannikin]

Well, we won't have roles in part 1, we add them in part 2. But, any calls to requireAuth() also won't have the roles argument passed, so it will fall through to return true. We'll only start passing roles once we actually have them in the database, and then the real checks will kick in.

[cannikin]

How's this looking, could we merge? I heard a rumor @thedavidprice was thinking of doing a release and it'd be nice if this was in it, otherwise people going through Part 2 of the tutorial are gonna be very confused!

[dthyresson]

How's this looking, could we merge? I heard a rumor @thedavidprice was thinking of doing a release and it'd be nice if this was in it, otherwise people going through Part 2 of the tutorial are gonna be very confused!

I need to check that the types don’t give a warning. And see if it did if adding empty roles to currentUser (even empty) is ok.

[cannikin]

So the real role checking only kicks in if you make a call to hasRole({ roles }), so it won't matter if User has a roles property unless you start making that function call. Once you do, if user has no roles property then it'll return undefined, and if they do have a roles property, but it's empty, it'll return false since it won't include the role you're checking for. So all those cases should be correct.

But, this is now the same as the non-dbAuth auth.js template so if any logic changes need to be made here they'd need to be made there as well, right?

[dthyresson]

But, this is now the same as the non-dbAuth auth.js template so if any logic changes need to be made here they'd need to be made there as well, right?

Ok. Approved

[cannikin]

On latest test run @thedavidprice

@redwoodjs/cli: FATAL ERROR: Committing semi space failed. Allocation failed - JavaScript heap out of memory

😬

[cannikin]

I went ahead and merged this, but after a couple runs the Windows Node 16 test still fails with out-of-memory errors.