Add actions and external identifier for response users. #43
Conversation
| UserExternalID = apps.get_model('core', 'UserExternalID') | ||
|
|
||
| for inc in Incident.objects.all(): | ||
| inc.reporter = UserExternalID.objects.get(external_id=inc.reporter_tmp) |
There was a problem hiding this comment.
Do the UserExternalID's always exist?
There was a problem hiding this comment.
At this point we can't create UserExtrnalID's it is the responsibility of the other apps. From core we don't know where each id came from and can't associate them. The slack app must do it from it's migration.
There was a problem hiding this comment.
My question is: what happens here if the UserExternalID doesn't exist when we try to get it here?
There was a problem hiding this comment.
We would want it to fail or we would lose data. The alternative would be to create a placeholder user that would not be associated with an application.
core/models/user_external.py
Outdated
| from django.contrib.auth.models import User | ||
|
|
||
|
|
||
| class UserExternalID(models.Model): |
There was a problem hiding this comment.
Can we make this a little less formal? Maybe ExternalUser or ResponseUser?
There was a problem hiding this comment.
This is used to associate the Django user to the identifier of another application. This should be a one to many 'user' to external application id. I added the display name since we don't link the users immediately or enforce that it must be linked. The username is a more friendly in the ui without needing to query somewhere else. I will make the change if you insist but it was not intended to be a "user".
There was a problem hiding this comment.
I just think the name is a mouthful. LinkedUser or ExternalUser would be preferable.
slack/incident_commands.py
Outdated
| incident.end_time = datetime.now() | ||
| incident.save() | ||
|
|
||
| comms_channel.post_in_channel(f"The incident has been closed! 📖 -> 📕") |
There was a problem hiding this comment.
Can't fix/Won't fix.
adamlphillips
force-pushed
the
actions-and-users
branch
2 times, most recently
from
75ea49a to
7ba0323
Compare
| UserExternalID = apps.get_model('core', 'UserExternalID') | ||
|
|
||
| for inc in Incident.objects.all(): | ||
| inc.reporter = UserExternalID.objects.get(external_id=inc.reporter_tmp) |
There was a problem hiding this comment.
My question is: what happens here if the UserExternalID doesn't exist when we try to get it here?
core/models/user_external.py
Outdated
| from django.contrib.auth.models import User | ||
|
|
||
|
|
||
| class UserExternalID(models.Model): |
There was a problem hiding this comment.
I just think the name is a mouthful. LinkedUser or ExternalUser would be preferable.
slack/dialog_handlers.py
Outdated
| severity = submission['severity'] | ||
|
|
||
| name = get_user_profile(user_id)['name'] | ||
| reporter = GetOrCreateSlackUserExternalID(external_id=user_id, display_name=name)[0] |
There was a problem hiding this comment.
We never use anything but the zeroth return value from this function - can we just return that to avoid the [0] at the end?
adamlphillips
force-pushed
the
actions-and-users
branch
from
7ba0323 to
b2549c8
Compare
No description provided.