-
Notifications
You must be signed in to change notification settings - Fork 16
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
DIA-2895 GDPR consent expiration #505
Conversation
legIntCategories = try container.decodeIfPresent(Array.self, forKey: .legIntCategories) | ||
legIntVendors = try container.decodeIfPresent(Array.self, forKey: .legIntVendors) | ||
vendors = try container.decodeIfPresent(Array.self, forKey: .vendors) | ||
categories = try container.decodeIfPresent(Array.self, forKey: .categories) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simply removed self
from the code above.
var noon: Date { | ||
Calendar.current.date(bySettingHour: 12, minute: 0, second: 0, of: self)! | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Helper methods, used only on tests, so far.
case .gdpr: .gdpr | ||
case .ccpa: .ccpa | ||
case .ios14: .ios14 | ||
default: .unknown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No changed logic, simply removing unnecessary return
keyword and inlining things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know swift, but can you omit the return
keyword when you have to return a certain value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bohdan-go-wombat yes, if that's the only statement in a function. A switch statement kinda returns a value after the case :
, that's why it works in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bohdan-go-wombat it turns out older versions of Swift complained about that 😅 , reverted back to have the return
keyword.
self.state.gdpr?.categories = response.categories ?? getResponse?.gdpr?.categories | ||
self.storage.spState = self.state | ||
|
||
self.handleGPDRPostChoice(action, getResponse, response) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the code above to new method handleGDPRPostChoice
, for better readability.
Example/ConsentViewController_ExampleTests/SPClientCoordinator/SPClientCoordinatorSpec.swift
Show resolved
Hide resolved
Changed expectation from .consentedAll to .consentedAny because some vendors depend on the IDFA status to be accepted in order to be considered consented.
expirationDate
to response classesexpirationDate
toSPGDPRConsent
class.loadMessage
, set GDPR consent back to.empty
ifexiprationDate < today
CCPA will be implemented in a different PR.