ENT-8079 - Make DSC optional based on enterprise config #190
Conversation
zwidekalanga
force-pushed
the
zwidekalanga/ENT-8079/make-dsc-optional
branch
2 times, most recently
from
81d90df to
085850d
Compare
zwidekalanga
force-pushed
the
zwidekalanga/ENT-8079/make-dsc-optional
branch
from
085850d to
984df22
Compare
zwidekalanga
force-pushed
the
zwidekalanga/ENT-8079/make-dsc-optional
branch
from
984df22 to
156bfb9
Compare
zwidekalanga
force-pushed
the
zwidekalanga/ENT-8079/make-dsc-optional
branch
from
156bfb9 to
4a1b73c
Compare
| @@ -121,7 +151,11 @@ def _validate(self, transaction): | |||
|
|
|||
| Raises an exception when the transaction is missing required information | |||
| """ | |||
| enterprise_customer_data = self._get_enterprise_customer_data(transaction) | |||
| enable_data_sharing_consent = enterprise_customer_data.get('enable_data_sharing_consent', False) | |||
| for field in self.REQUIRED_METADATA_FIELDS: | |||
There was a problem hiding this comment.
[curious] I'm wondering if it'd make sense to conditionally push geag_data_share_consent onto self.REQUIRED_METADATA_FIELDS when enable_data_sharing_consent = True, so that the fields listed in self.REQUIRED_METADATA_FIELDS are truly only the required fields?
There was a problem hiding this comment.
The issue is related to the tests since they share they same object adding more removing into the REQUIRED_METADATA_FIELDS array in causes other tests to fail which where support to pass.
There was a problem hiding this comment.
Got it. I suppose the other concern is that I'm not sure we'd be able to get at the appropriate enterprise customer within a class constructor, as we are relying on a specific transaction to identify the enterprise customer. I think how you have it is reasonable.
| @@ -121,7 +151,11 @@ def _validate(self, transaction): | |||
|
|
|||
| Raises an exception when the transaction is missing required information | |||
| """ | |||
| enterprise_customer_data = self._get_enterprise_customer_data(transaction) | |||
| enable_data_sharing_consent = enterprise_customer_data.get('enable_data_sharing_consent', False) | |||
| for field in self.REQUIRED_METADATA_FIELDS: | |||
There was a problem hiding this comment.
Got it. I suppose the other concern is that I'm not sure we'd be able to get at the appropriate enterprise customer within a class constructor, as we are relying on a specific transaction to identify the enterprise customer. I think how you have it is reasonable.
Description
Describe in a couple of sentences what this PR adds
Testing instructions
Add some, if applicable
Merge checklist