Skip to content

Loading…

Bug 976835: Only offer fully reviewed versions as updates to fully revie... #1809

Closed
wants to merge 1 commit into from

2 participants

@kmaglione

...wed versions.

@ngokevin
Mozilla member

Let's move this PR to Olympia, the new AMO repo.

@ngokevin
Mozilla member

Closing since this PR is on Olympia, which will be deployed on April 1st. Re-open if it really needs to get out before then.

@ngokevin ngokevin closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 25, 2014
  1. @kmaglione
This page is out of date. Refresh to see the latest.
Showing with 45 additions and 53 deletions.
  1. +3 −4 apps/addons/models.py
  2. +16 −0 apps/addons/tests/test_update.py
  3. +26 −49 services/update.py
View
7 apps/addons/models.py
@@ -675,10 +675,9 @@ def get_version(self, backup_version=False):
return self.versions.no_cache().filter(**fltr).extra(
where=["""
NOT EXISTS (
- SELECT 1 FROM versions as v2
- INNER JOIN files AS f2 ON (f2.version_id = v2.id)
- WHERE v2.id = versions.id
- AND f2.status NOT IN (%s))
+ SELECT 1 FROM files AS f2
+ WHERE f2.version_id = versions.id AND
+ f2.status NOT IN (%s))
""" % status_list])[0]
except (IndexError, Version.DoesNotExist):
View
16 apps/addons/tests/test_update.py
@@ -368,11 +368,25 @@ def test_file_preliminary_addon(self):
for status in amo.LITE_STATUSES:
self.addon.update(status=status)
+ self.change_status(self.version_1_2_0, amo.STATUS_LITE)
self.change_status(self.version_1_2_1, amo.STATUS_LITE)
version, file = self.get('1.2', self.version_int,
self.app, amo.PLATFORM_LINUX)
eq_(version, self.version_1_2_1)
+ def test_file_preliminary_ex_full_addon(self):
+ """
+ If the addon is in prelim. review, user has a full reviewed version.
+ Show the most recent full reviewed version.
+ """
+ self.addon.update(status=amo.STATUS_LITE)
+
+ self.change_status(self.version_1_2_0, amo.STATUS_PUBLIC)
+ self.change_status(self.version_1_2_2, amo.STATUS_LITE)
+ version, file = self.get('1.2', self.version_int,
+ self.app, amo.PLATFORM_LINUX)
+ eq_(version, self.version_1_2_1)
+
class TestDefaultToCompat(amo.tests.TestCase):
"""
@@ -663,6 +677,8 @@ def test_beta_version(self):
version.version = beta_version
version.save()
+ version.addon.update(status=amo.STATUS_PUBLIC)
+
data = self.good_data.copy()
up = self.get(data)
up.is_valid()
View
75 services/update.py
@@ -143,45 +143,12 @@ def is_valid(self):
self.is_beta_version = base.VERSION_BETA.search(data['version'])
return True
- def get_beta(self):
- data = self.data
- data['status'] = base.STATUS_PUBLIC
-
- if data['addon_status'] == base.STATUS_PUBLIC:
- # Beta channel looks at the addon name to see if it's beta.
- if self.is_beta_version:
- # For beta look at the status of the existing files.
- sql = """
- SELECT versions.id, status
- FROM files INNER JOIN versions
- ON files.version_id = versions.id
- WHERE versions.addon_id = %(id)s
- AND versions.version = %(version)s LIMIT 1;"""
- self.cursor.execute(sql, data)
- result = self.cursor.fetchone()
- # Only change the status if there are files.
- if result is not None:
- status = result[1]
- # If it's in Beta or Public, then we should be looking
- # for similar. If not, find something public.
- if status in (base.STATUS_BETA, base.STATUS_PUBLIC):
- data['status'] = status
- else:
- data.update(STATUSES_PUBLIC)
- self.flags['multiple_status'] = True
-
- elif data['addon_status'] in (base.STATUS_LITE,
- base.STATUS_LITE_AND_NOMINATED):
- data['status'] = base.STATUS_LITE
- else:
- # Otherwise then we'll keep the update within the current version.
- data['status'] = base.STATUS_NULL
- self.flags['use_version'] = True
-
def get_update(self):
- self.get_beta()
data = self.data
+ data.update(STATUSES_PUBLIC)
+ data['STATUS_BETA'] = base.STATUS_BETA
+
sql = ["""
SELECT
addons.guid as guid, addons.addontype_id as type,
@@ -212,17 +179,27 @@ def get_update(self):
if data.get('appOS'):
sql.append(' OR files.platform_id = %(appOS)s')
- if self.flags['use_version']:
- sql.append(') WHERE files.status > %(status)s AND '
- 'versions.version = %(version)s ')
- else:
- if self.flags['multiple_status']:
- # Note that getting this properly escaped is a pain.
- # Suggestions for improvement welcome.
- sql.append(') WHERE files.status in (%(STATUS_PUBLIC)s,'
- '%(STATUS_LITE)s,%(STATUS_LITE_AND_NOMINATED)s) ')
- else:
- sql.append(') WHERE files.status = %(status)s ')
+ sql.append("""
+ )
+ LEFT JOIN versions curver
+ ON curver.addon_id = addons.id AND curver.version = %(version)s
+ LEFT JOIN files curfile
+ ON curfile.version_id = curver.id
+ WHERE
+ CASE
+ WHEN curfile.status = %(STATUS_BETA)s
+ THEN
+ addons.status = %(STATUS_PUBLIC)s AND
+ files.status = %(STATUS_BETA)s
+ WHEN addons.status IN (%(STATUS_LITE)s,
+ %(STATUS_LITE_AND_NOMINATED)s)
+ AND (curfile.id IS NULL OR curfile.status = %(STATUS_LITE)s)
+ THEN
+ files.status = %(STATUS_LITE)s
+ ELSE
+ files.status = %(STATUS_PUBLIC)s
+ END
+ """)
sql.append('AND appmin.version_int <= %(version_int)s ')
@@ -272,8 +249,8 @@ def get_update(self):
'version', 'premium_type'],
list(result)))
row['type'] = base.ADDON_SLUGS_UPDATE[row['type']]
- row['url'] = get_mirror(self.data['addon_status'],
- self.data['id'], row)
+ row['url'] = get_mirror(data['addon_status'],
+ data['id'], row)
data['row'] = row
return True
Something went wrong with that request. Please try again.