Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: maintain dashes and case during scope arithmetic #25

Merged
merged 2 commits into from
Aug 11, 2020

Conversation

cqr
Copy link
Contributor

@cqr cqr commented Aug 11, 2020

Primarily aesthetic, since code that checks for scopes should never be looking at string values directly, but having this work this way would have made some tests I have been writing much easier.

@cqr cqr requested a review from cavis August 11, 2020 11:17
Copy link
Member

@cavis cavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 some bonus spaces, but i think this looks good.

test/prx_auth/scope_list_test.rb Outdated Show resolved Hide resolved

it 'maintains dashes and capitalization in the result' do
sl = new_list('The-Beginning the-middle the-end') - new_list('the-Middle')
assert sl.to_s == 'The-Beginning the-end'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so is this why tokens now have read_private in their "scope" instead of read-private? Not that we ever look for those anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that's why (though they are equivalent for the purposes of checking if a scope is present in a token, the string representation is different so pulling a list in tests was slightly more of a pain).

@cavis cavis merged commit 63c306c into master Aug 11, 2020
@cavis cavis deleted the fix/case_and_dash branch August 11, 2020 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants