Feature custom grant types #37
Conversation
Ensuring string is not an authorizable object anymore
flbulgarelli
force-pushed
the
feature-custom-grant-types
branch
from
319c607 to
26ea92d
Compare
| grant = grant_like.to_mumukit_grant | ||
| case grant | ||
| when FirstPartGrant then grant.first == first | ||
| when SingleGrant then grant.slug.first == first |
There was a problem hiding this comment.
We could delegate first to slug in SingleGrant and compact this two lines like:
when FirstPartGrant, SingleGrant then grant.first == first
| true | ||
| end | ||
|
|
||
| def includes?(_) |
There was a problem hiding this comment.
_grant_like to make it more explicit?
| @@ -114,7 +133,7 @@ def match(pattern, part) | |||
|
|
|||
| def self.validate_slug!(slug) | |||
| unless slug =~ /\A[^\/\n]+\/[^\/\n]+\z/ | |||
There was a problem hiding this comment.
maybe we should use SLUG_REGEXP here now?
There was a problem hiding this comment.
This one is checking for SOMETHING/SOMETHING while the other one is checking the validity of that SOMETHING as far as I understand. Using SLUG_REGEXP as part of this validation, wouldn't make this one more restrictive and the other validations obsolete? (You are the king of regex, so correct me if I'm wrong).
I do agree that it feels quite odd that the validations are distributed between this method and the initialization.
There was a problem hiding this comment.
I do agree that it feels quite odd that the validations are distributed between this method and the initialization.
right, that's what I was going for, it seems odd that we're doing two different regexp validations over the same thing. We could end up with cases where, for a given slug, initialize fails but validate_slug! passes... isn't that weird?
There was a problem hiding this comment.
I was thinking we could just replace the [^\/\n] parts in this regex with a SLUG_REGEXP interpolation, but I just noticed that would cause trouble because SLUG_REGEXP is anchored... maybe we could do something like
SLUG_PART = /[[[:alnum:]]_ .-]+/
ANCHORED_SLUG_PART = /^#{SLUG_PART}$/
SLUG_REGEXP = /^#{SLUG_PART}\/#{SLUG_PART}$/There was a problem hiding this comment.
I am not even sure we actually need this method 🤷♂️
|
@flbulgarelli This PR is very close to be finished. |
|
Go ahead! I don't think i will be able to work on it soon |
This PR refactors in a non backward-compatible way some important slugs and grants semantics:
to_mumukit_slugwas removed*/*-like strings are not a valid slugs anymore_/_-like strings are not valid grants anymorePermissions.has_permissions?has been deprecatedallows?andincludes?authorized_by?is now provided as a way to use slugs and grant polymorphically, using a double dispatchgranted_organizationsis provided as a way to tell which are the explicitly granted organizations for a grant, instead of depending onto_mumukit_slug