Skip to content
This repository was archived by the owner on Dec 20, 2018. It is now read-only.

Conversation

HaoK
Copy link
Member

@HaoK HaoK commented Oct 29, 2018

Fix for #2034

@HaoK
Copy link
Member Author

HaoK commented Oct 29, 2018

I tried out the code via scaffolding the fix into a 2.1 app and it seems fine:

image

This broke a bunch of functional tests so we actually do have coverage, the tests now have some minimal coverage for enabling 2fa behavior with and without cookie consent now.

@Eilon @blowdart @ajcvickers

Copy link
Contributor

@ajcvickers ajcvickers left a comment

Choose a reason for hiding this comment

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

But I'm not a UI expert...

@blowdart
Copy link
Member

What happens if the dev removed all the consent code?

@HaoK
Copy link
Member Author

HaoK commented Oct 30, 2018

They would need to remove this as well I assume? Or does CanTrack cover that case?

@blowdart
Copy link
Member

I don't know, that's what I'm asking. I would have hoped the check defaulted to true if there was no consent feature, but ...

@HaoK
Copy link
Member Author

HaoK commented Oct 30, 2018

Are you telling me CanTrack returns false by default? @Tratcher any insight into the behavior?

@HaoK
Copy link
Member Author

HaoK commented Oct 30, 2018

Updated to assume if no consent feature is there, that means we are good to go (and to allow 2fa)

Copy link
Contributor

@Eilon Eilon left a comment

Choose a reason for hiding this comment

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

Just 1 tiny comment on something that was already there. Otherwise good.

@HaoK HaoK merged commit da9318f into release/2.2 Oct 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants