Save organisation transcript preferences - EDUCATOR-1048 #14
Conversation
mushtaqak
force-pushed
the
mushtaq/save-org-transcript-prefs
branch
from
163e8c0 to
c5e1ad1
Compare
VEDA_OS01/models.py
Outdated
| """ | ||
| Model to contain third party transcription service provider preferances. | ||
| """ | ||
| org = models.CharField('Organization', max_length=50, unique=True) |
There was a problem hiding this comment.
can we add help text to tell others that these are organizations from studio / platform, not the institutes in VEDA.
VEDA_OS01/models.py
Outdated
| api_key = models.CharField('API key', max_length=255) | ||
| api_secret = models.CharField('API secret', max_length=255, null=True, blank=True) | ||
| created = models.DateTimeField(auto_now_add=True, null=True) | ||
| modified = models.DateTimeField(auto_now=True, null=True) |
There was a problem hiding this comment.
can we use timestamped model which includes these fields by default ?
There was a problem hiding this comment.
timestamped is not available in this repo. we can either add in or just go with these 2 fields for now.
There was a problem hiding this comment.
Let's add Django model utils to use timestamped model.
VEDA_OS01/models.py
Outdated
| Model to contain third party transcription service provider preferances. | ||
| """ | ||
| org = models.CharField('Organization', max_length=50, unique=True) | ||
| provider = models.CharField('Transcription provider', max_length=50, choices=TranscriptionProviderType.CHOICES) |
There was a problem hiding this comment.
@mushtaqak I think we need to make org & provider unique in a combination. So that orgs can have both credentials to use them in different courses.
VEDA_OS01/models.py
Outdated
| modified = models.DateTimeField(auto_now=True, null=True) | ||
|
|
||
| def __unicode__(self): | ||
| return self.org |
There was a problem hiding this comment.
I would add org & provider as unicode. Also, can we make this real unicode.
VEDA_OS01/models.py
Outdated
| ) | ||
|
|
||
|
|
||
| class TranscriptionProvider(models.Model): |
There was a problem hiding this comment.
Can we name it TranscriptionPreferences ?
|
@yro I can't see migration in edx-video-pipeline, is it by intention ??? Also, how they work on production ? |
| @@ -137,6 +137,7 @@ | |||
| 'rest_framework.authtoken', | |||
| 'oauth2_provider', | |||
| 'rest_framework', | |||
| 'django_filters', | |||
There was a problem hiding this comment.
Note to reviewers: This is added to resolve TemplateNotExist error when accessing rest api views.
See https://github.com/edx/credentials/pull/284/files and carltongibson/django-filter#562
|
@mushtaqak We don't have a migration folder yet. I'll do the migrations (via manage.py) for this on deploy and check the folder back into the repo for future use. Good catch! |
|
@yro Do you think we should merge the changes related to models in master branch without migrations. I'm not sure just because of migrations on prod and how they'll work. |
|
@muzaffaryousaf yes you should merge without migrations just this one time. After I push this PR to prod, I'll pull the migrations back into the repo, and future changes to models/migrations can be made in the repo. |
VEDA_OS01/models.py
Outdated
| ) | ||
|
|
||
| class Meta: | ||
| unique_together = ('video', 'provider', 'lang_code') |
There was a problem hiding this comment.
TODO: we need to remove this constraint, this will raise integrity in case of VEDA heal (i.e. transcription re-process for same lang)
muzaffaryousaf
changed the base branch from
master
to
transcripts-3rd-party-integration
…during file discovery
muzaffaryousaf
force-pushed
the
mushtaq/save-org-transcript-prefs
branch
from
3b2cdb5 to
2cc7eab
Compare
The purpose of this PR is to save organisation transcript preferences and show in admin side.
EDUCATOR-1048