Prod release #783
Prod release #783
Conversation
…ectory-api into MVP-395-cache-com-trade-data
Mvp 395 cache com trade data
…16-recomment-events
Personalise events by user sector and country of interest
allow brand product details
Post deployment changelog update
…-cache-task MVP-474 add tasks for caching comtrade
mvp-581 added new fields
…nies MVP-432 fixed adding countries error
…nstants GP2-699-swamp-route-to-market-constants
* Bump directory-sso-client version (#763) * fix cc when empty (#764) * update constants * show route to market admin (#767) * Added Countries and SeletedCountries Model and relevant data (#768) * Staging release (#765) (#769) * Bump directory-sso-client version (#763) * fix cc when empty (#764) Co-authored-by: Gurdeep Atwal <44236490+depsiatwal@users.noreply.github.com> Co-authored-by: Gurdeep Atwal <44236490+depsiatwal@users.noreply.github.com> Co-authored-by: Gurdeep Atwal <44236490+depsiatwal@users.noreply.github.com> Co-authored-by: gatwal <gurdeep.atwal@digital.trade.gov.uk> Co-authored-by: Robert Meredith <bob.meredith@ntlworld.com>
staging release
New hawk authenticated endpoint at /activity-stream/companies
…tream-endpoint Add companies endpoint for activity stream
Added new env var for API's request key
…arket-data Added endpoint for target market data
es upgrade
staging release
![code-review-doctor[bot]](https://avatars.githubusercontent.com/in/83430?s=60&v=4){kind=small}
| class HsCodeSector(models.Model): | ||
| hs_code = models.CharField(max_length=10) | ||
| product = models.TextField() | ||
| sector = models.CharField(max_length=255, blank=True, null=True) |
There was a problem hiding this comment.
| sector = models.CharField(max_length=255, blank=True, null=True) | |
| sector = models.CharField(max_length=255, blank=True, default='') |
null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="".
|
|
||
|
|
||
| class WorldEconomicOutlook(TimeStampedModel): | ||
| country_code = models.CharField(unique=False, blank=False, null=False, max_length=50) |
There was a problem hiding this comment.
| country_code = models.CharField(unique=False, blank=False, null=False, max_length=50) | |
| country_code = models.CharField(max_length=50) |
False is the default value Django uses for unique, so unique=False can be removed.
As above, redundant default arguments can be removed.
As above, redundant default arguments can be removed.
|
|
||
| class WorldEconomicOutlook(TimeStampedModel): | ||
| country_code = models.CharField(unique=False, blank=False, null=False, max_length=50) | ||
| country_name = models.CharField(unique=False, blank=False, null=False, max_length=255) |
There was a problem hiding this comment.
| country_name = models.CharField(unique=False, blank=False, null=False, max_length=255) | |
| country_name = models.CharField(max_length=255) |
Likewise, redundant default arguments can be removed.
Same as above: redundant default arguments can be removed.
Likewise, redundant default arguments can be removed.
| country_code = models.CharField(unique=False, blank=False, null=False, max_length=50) | ||
| country_name = models.CharField(unique=False, blank=False, null=False, max_length=255) | ||
| subject = models.CharField(unique=False, blank=False, null=False, max_length=100) | ||
| scale = models.CharField(unique=False, blank=False, null=False, max_length=100) |
There was a problem hiding this comment.
| scale = models.CharField(unique=False, blank=False, null=False, max_length=100) | |
| scale = models.CharField(max_length=100) |
As above, redundant default arguments can be removed.
As above, redundant default arguments can be removed.
Again, redundant default arguments can be removed.
There was a problem hiding this comment.
oh dear it's duplicating the messages :(
There was a problem hiding this comment.
hahah @richtier don't worry iterative process
| promotion_channels = JSONField(null=True, blank=True, default=list) | ||
| resource_needed = models.TextField(null=True, blank=True, default='', validators=[no_html]) | ||
| spend_marketing = models.FloatField(null=True, default=None, unique=False) | ||
|
|
||
|
|
||
| class CompanyObjectives(TimeStampedModel): | ||
| description = models.TextField(null=True, blank=True, default='', validators=[no_html]) | ||
| owner = models.PositiveIntegerField(null=True, verbose_name='sso user.sso_id', default=None, unique=False) | ||
| planned_reviews = models.TextField(blank=True, default='', validators=[no_html]) | ||
| owner = models.TextField(null=True, blank=True, max_length=100) |
There was a problem hiding this comment.
| owner = models.TextField(null=True, blank=True, max_length=100) | |
| owner = models.TextField(default='', blank=True, max_length=100) |
null=True on a string field causes inconsistent data types because the value can be either str or None. This adds complexity and maybe bugs, but can be solved by replacing null=True with default="".
| companyexportplan = models.ForeignKey( | ||
| CompanyExportPlan, related_name='company_objectives', on_delete=models.CASCADE | ||
| ) | ||
|
|
||
|
|
||
| class RouteToMarkets(TimeStampedModel): | ||
| route = models.CharField(max_length=30, blank=True, null=True, default='', choices=choices.MARKET_ROUTE_CHOICES) |
There was a problem hiding this comment.
Same as above: consider replacing null=True with default="" (and blank=True to pass validation checks).
| companyexportplan = models.ForeignKey( | ||
| CompanyExportPlan, related_name='company_objectives', on_delete=models.CASCADE | ||
| ) | ||
|
|
||
|
|
||
| class RouteToMarkets(TimeStampedModel): | ||
| route = models.CharField(max_length=30, blank=True, null=True, default='', choices=choices.MARKET_ROUTE_CHOICES) | ||
| promote = models.CharField( |
There was a problem hiding this comment.
As above, consider replacing null=True with default="" (and blank=True to pass validation checks).
| country = models.ForeignKey( | ||
| 'dataservices.Country', | ||
| verbose_name=_('Suggested Countries'), | ||
| on_delete=models.SET_NULL, | ||
| null=True | ||
| ) |
There was a problem hiding this comment.
| country = models.ForeignKey( | |
| 'dataservices.Country', | |
| verbose_name=_('Suggested Countries'), | |
| on_delete=models.SET_NULL, | |
| null=True | |
| ) | |
| country = models.ForeignKey('dataservices.Country', verbose_name=_('Suggested Countries'), on_delete=models.SET_NULL, null=True, blank=True) |
Django automatically creates a related_name if it's not set. If it were set then a more readable and explicit relationship is set up.
Expect unwanted behavior if null and blank are different values: null controls if the database allows no value for country and blank controls if the application allows no value for country. Consider setting null and blank to the same value for country.
| def __str__(self): | ||
| return str(self.hs_code) | ||
|
|
||
| class Meta: |
There was a problem hiding this comment.
According to Django internal style guide, Meta should come before __str__.
Codecov Report
@@ Coverage Diff @@
## master #783 +/- ##
=========================================
Coverage ? 97.02%
=========================================
Files ? 107
Lines ? 3467
Branches ? 0
=========================================
Hits ? 3364
Misses ? 103
Partials ? 0
Continue to review full report at Codecov.
|
Prod release