Sprint2__Subscriber Features 2 of 2 (Client dependent)#44
Sprint2__Subscriber Features 2 of 2 (Client dependent)#44sharma-sagar wants to merge 2 commits intofacebookarchive:masterfrom
Conversation
Negative/Formated Transaction IDs for Usage Events Block/Unblock Subscriber on Invalid Events
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions. |
kkroo
left a comment
There was a problem hiding this comment.
some comments about implementing the 24 hour event window
| ) | ||
| # These UsageEvent events are not allowed block the Subscriber if repeated | ||
| # for 3 times | ||
| INVALID_EVENTS = ('error_call', 'error_sms') |
There was a problem hiding this comment.
We should keep this list empty/commented out until we define the use cases and implement/validate the dialplan/chatplan event generation
There was a problem hiding this comment.
yes, we have kept this as dummy for demo, as soon that functionality gets added we will replace with new EVENT.
| max_length=255) | ||
| block_time = models.DateTimeField(null=True, blank=True) | ||
|
|
||
| class Meta: |
There was a problem hiding this comment.
why are there new permissions introduced here?
| max_transactions = event.subscriber.network.max_failure_transaction | ||
| if subscriber_event.count == max_transactions: | ||
| subscriber.is_blocked = True | ||
| subscriber.block_reason = 'Repeated %s within 24 hours ' % ( |
There was a problem hiding this comment.
do we need block reason? Can we just list out the negative_transactions in the UI?
There was a problem hiding this comment.
Block reason is kept for REPORTING purpose, and we are already listing negative transactions.
There was a problem hiding this comment.
if we need the reports, we have the usageevents too
| negative_transactions_ids = subscriber_event.negative_transactions + [ | ||
| event.transaction_id] | ||
| subscriber_event.count = subscriber_event.count + 1 | ||
| subscriber_event.event_time = event.date |
There was a problem hiding this comment.
we should only update the event_time of the first event so you can do the 24 hour check
| subscriber_event.save() | ||
|
|
||
| max_transactions = event.subscriber.network.max_failure_transaction | ||
| if subscriber_event.count == max_transactions: |
| print 'Unblocking subscribers %s blocked for past 24 hours' % ( | ||
| [subscriber.imsi for subscriber in subscribers], ) | ||
| subscribers.update(is_blocked=False, block_time=None, | ||
| block_reason='No reason to block yet!') No newline at end of file |
There was a problem hiding this comment.
we should also have another task to delete SubscriberInvalidEvents that are older than 24 hours? it seems like it would also need to update the event_time to the time of the 2nd invalid event in the negative actions list.
There was a problem hiding this comment.
We are intentionally not deleting SubscriberInvalidEvents for reporting purpose.
Though it gets replaced after 24 hours by subscriber again if any negative transaction occur.
We are not updating 2nd invalid event time, as if we do we won't be able to calculate the 24 hour time span of the transactions for that particular subscriber.
Also if we need details of that transaction it can be captured from UsageEvent table as the transaction id remains same.
There was a problem hiding this comment.
Its still not clear how we are blocking the subscriber only if it is a 3rd event in 24hr block. Can you perhaps explain the flow?
|
@sagarsharma-aricent updated the pull request - view changes |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Please review Subscriber features which include:
--> negative (for invalid events) and formatted transaction IDs for Usage Events
--> block/unblock Subscriber on Invalid Events