Merge branch 'bug-683871-id_number_fix'

Conflicts:
	apps/jetpack/views.py

apps/jetpack/models.py

@@ -280,7 +280,7 @@ class PackageRevision(BaseModel):
     ###############
 
     def get_cache_hashtag(self):
-        return "%sr%d" % (self.package.id_number, self.revision_number)
+        return "revision-%s" % self.pk
 
     # NAME and FULL_NAME in Revision #############
 
@@ -369,43 +369,31 @@ class PackageRevision(BaseModel):
             if self.package.version.revision_number == self.revision_number:
                 return self.package.get_absolute_url()
             return reverse(
-                'jp_%s_version_details' \
-                % settings.PACKAGE_SINGULAR_NAMES[self.package.type],
-                args=[self.package.id_number, self.version_name])
+                'jp_version_details',
+                args=[self.package.pk, self.version_name])
         return reverse(
-            'jp_%s_revision_details' \
-            % settings.PACKAGE_SINGULAR_NAMES[self.package.type],
-            args=[self.package.id_number, self.revision_number])
+            'jp_revision_details',
+            args=[self.package.pk, self.revision_number])
 
     def get_save_url(self):
         " returns URL to save the package revision "
-        return reverse(
-            'jp_%s_revision_save' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_revision_save', args=[self.pk])
 
     def get_add_module_url(self):
         " returns URL to add module to the package revision "
-        return reverse(
-            'jp_%s_revision_add_module' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_add_module', args=[self.pk])
 
     def get_rename_module_url(self):
         " returns URL to rename module in the package revision "
-        return reverse(
-            'jp_%s_revision_rename_module' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_rename_module', args=[self.pk])
 
     def get_remove_module_url(self):
         " returns URL to remove module from the package revision "
-        return reverse(
-            'jp_%s_revision_remove_module' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_remove_module', args=[self.pk])
 
     def get_upload_attachment_url(self):
         " returns URL to upload attachment to the package revision "
-        return reverse(
-            'jp_%s_revision_upload_attachment' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_upload_attachment', args=[self.pk])
 
     def get_add_attachment_url(self):
         " returns URL to add attachment to the package revision "
@@ -413,49 +401,35 @@ class PackageRevision(BaseModel):
 
     def get_rename_attachment_url(self):
         " returns URL to rename module in the package revision "
-        return reverse(
-            'jp_%s_revision_rename_attachment' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_rename_attachment', args=[self.pk])
 
     def get_remove_attachment_url(self):
         " returns URL to remove attachment from the package revision "
-        return reverse(
-            'jp_%s_revision_remove_attachment' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_remove_attachment', args=[self.pk])
 
     def get_assign_library_url(self):
         " returns url to assign library to the package revision "
-        return reverse(
-            'jp_%s_revision_assign_library' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_assign_library', args=[self.pk])
 
     def get_update_library_url(self):
         " returns url to update library to a specific version "
-        return reverse(
-            'jp_%s_revision_update_library' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_update_library', args=[self.pk])
 
     def get_remove_library_url(self):
         " returns url to remove library from the package revision "
-        return reverse(
-            'jp_%s_revision_remove_library' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_remove_library', args=[self.pk])
 
     def get_test_xpi_url(self):
         " returns URL to test Add-on "
         if self.package.type != 'a':
             raise Exception('XPI might be created only from an Add-on')
-        return reverse(
-            'jp_addon_revision_test',
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_addon_revision_test', args=[self.pk])
 
     def get_download_xpi_url(self):
         " returns URL to download Add-on's XPI "
         if self.package.type != 'a':
             raise Exception('XPI might be created only from an Add-on')
-        return reverse(
-            'jp_addon_revision_xpi',
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_addon_revision_xpi', args=[self.pk])
 
     def get_upload_to_amo_url(self):
         " returns URL to upload to AMO "
@@ -467,30 +441,20 @@ class PackageRevision(BaseModel):
 
     def get_copy_url(self):
         " returns URL to copy the package "
-        return reverse(
-            'jp_%s_revision_copy' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_copy', args=[self.pk])
 
     def get_switch_sdk_url(self):
         " returns URL to switch SDK on the package revision "
-        return reverse(
-            'jp_addon_switch_sdk_version',
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_addon_switch_sdk_version', args=[self.pk])
 
     def get_add_folder_url(self):
-        return reverse(
-            'jp_%s_revision_add_folder' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_add_folder', args=[self.pk])
 
     def get_remove_folder_url(self):
-        return reverse(
-            'jp_%s_revision_remove_folder' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_revision_remove_folder', args=[self.pk])
 
     def get_latest_dependencies_url(self):
-        return reverse(
-            'jp_%s_check_latest_dependencies' % self.package.get_type_name(),
-            args=[self.package.id_number, self.revision_number])
+        return reverse('jp_package_check_latest_dependencies', args=[self.pk])
 
     def get_modules_list_url(self):
         return reverse('jp_revision_get_modules_list', args=[self.pk])
@@ -554,7 +518,7 @@ class PackageRevision(BaseModel):
             'description': escape(self.package.description),
             'author': self.package.author.get_profile().get_nickname(),
             'id': self.package.jid if self.package.is_addon() \
-                    else self.package.id_number,
+                    else self.package.pk,
             'version': version,
             'main': self.module_main,
             'dependencies': self.get_dependencies_list(sdk),
@@ -768,7 +732,7 @@ class PackageRevision(BaseModel):
         """
         revision_numbers = PackageRevision.objects.filter(
                                     author__username=self.author.username,
-                                    package__id_number=self.package.id_number
+                                    package__pk=self.package.pk
                                 ).order_by('-revision_number')
         return revision_numbers[0].revision_number + 1 \
                 if revision_numbers else 1
@@ -1112,7 +1076,7 @@ class PackageRevision(BaseModel):
 
         # a LibraryRevision can't depend on another LibraryRevision
         # linked with the same Library
-        if dep.package.id_number == self.package.id_number:
+        if dep.package.pk == self.package.pk:
             raise SelfDependencyException(
                 'A Library cannot depend on itself!')
 
@@ -1287,10 +1251,7 @@ class PackageRevision(BaseModel):
         " returns modules list as JSON object "
         return [{
                 'path': m.filename,
-                'get_url': reverse('jp_get_module', args=[
-                    self.package.id_number,
-                    self.revision_number,
-                    m.filename])
+                'get_url': reverse('jp_get_module', args=[self.pk, m.filename])
                 } for m in self.modules.all()
             ] if self.modules.count() > 0 else []
 
@@ -1316,7 +1277,8 @@ class PackageRevision(BaseModel):
     def get_sdk_name(self):
         " returns the name of the directory to which SDK should be copied "
         return '%s-%s-%s' % (self.sdk.version,
-                             self.package.id_number, self.revision_number)
+                             self.package.pk,
+                             self.revision_number)
 
     def get_sdk_dir(self, hashtag):
         " returns the path to the directory where the SDK should be copied "
@@ -1712,13 +1674,11 @@ class Package(BaseModel, SearchMixin):
 
     def get_absolute_url(self):
         " returns the URL View Source "
-        return reverse('jp_%s_details' % self.get_type_name(),
-                        args=[self.id_number])
+        return reverse('jp_details', args=[self.pk])
 
     def get_latest_url(self):
         " returns the URL to view the latest saved Revision "
-        return reverse('jp_%s_latest' % self.get_type_name(),
-                        args=[self.id_number])
+        return reverse('jp_latest', args=[self.pk])
 
     def get_latest_revision_number_url(self):
         " returns url to get the latest revision number "
@@ -1726,18 +1686,15 @@ class Package(BaseModel, SearchMixin):
 
     def get_disable_url(self):
         " returns URL to the disable package functionality "
-        return reverse('jp_package_disable',
-                        args=[self.id_number])
+        return reverse('jp_package_disable', args=[self.pk])
 
     def get_activate_url(self):
         " returns URL to activate disabled package "
-        return reverse('jp_package_activate',
-                        args=[self.id_number])
+        return reverse('jp_package_activate', args=[self.pk])
 
     def get_delete_url(self):
         " returns URL to delete package "
-        return reverse('jp_package_delete',
-                        args=[self.id_number])
+        return reverse('jp_package_delete', args=[self.pk])
 
     def get_view_on_amo_url(self):
         " returns the url to view the add-on on AMO "

apps/jetpack/package_helpers.py

@@ -14,17 +14,22 @@ from jetpack.errors import ManifestNotValid
 log = commonware.log.getLogger('f.package_helpers')
 
 
-def get_package_revision(id_name, type_id,
+def get_package_revision(pk=None, id_name=None, type_id=None,
                          revision_number=None,
                          version_name=None, latest=False):
     """
     Return revision of the package
     """
+    if not pk and not id_name:
+        raise Http404
 
     if not (revision_number or version_name):
         # get default revision - one linked via Package:version
-        package = get_object_with_related_or_404(Package, id_number=id_name,
-                                                 type=type_id)
+        if pk:
+            package = get_object_with_related_or_404(Package, pk=pk)
+        else:
+            package = get_object_with_related_or_404(Package,
+                    id_number=id_name, type=type_id)
         package_revision = package.latest if latest else package.version
         if not package_revision:
             log.critical("Package %s by %s has no latest or version "
@@ -33,12 +38,20 @@ def get_package_revision(id_name, type_id,
 
     elif revision_number:
         # get version given by revision number
-        package_revision = get_object_with_related_or_404(PackageRevision,
+        if pk:
+            package_revision = get_object_with_related_or_404(PackageRevision,
+                            package__pk=pk, revision_number=revision_number)
+        else:
+            package_revision = get_object_with_related_or_404(PackageRevision,
                             package__id_number=id_name, package__type=type_id,
                             revision_number=revision_number)
     elif version_name:
         # get version given by version name
-        package_revision = get_object_with_related_or_404(PackageRevision,
+        if pk:
+            package_revision = get_object_with_related_or_404(PackageRevision,
+                            package__pk=pk, version_name=version_name)
+        else:
+            package_revision = get_object_with_related_or_404(PackageRevision,
                             package__id_number=id_name, package__type=type_id,
                             version_name=version_name)
     # For unknown reason some revisions are not linked to any package

apps/jetpack/templates/_sidebar_revision_libraries_list.html

@@ -1,6 +1,6 @@
 {% for library in libraries %}
 	<li class="UI_File_Normal">
-		<a title="" id="library_{{ library.name }}" href="{{ library.get_absolute_url()}}" target="{{ library.package.id_number }}" class="library_link">
+		<a title="" id="library_{{ library.name }}" href="{{ library.get_absolute_url()}}" target="window_{{ library.package.pk }}" class="library_link">
 			{{ library.full_name }}
 			{% if not readonly %}<span class="File_close"></span>{% endif %}
 		</a>

apps/jetpack/templates/js/_package_initiate.js

@@ -37,5 +37,5 @@ attachments: {{ revision.get_attachments_list_json()|safe }},
 folders: {{ revision.get_folders_list_json()|safe }},
 // Actions
 copy_url: '{{ revision.get_copy_url() }}',
-revisions_list_html_url: '{{ url('jp_revisions_list_html', revision.package.id_number) }}/{revision_number}/',
+revisions_list_html_url: '{{ url('jp_revisions_list_html', revision.pk) }}',
 check_if_latest: {{ revision.is_latest|yesno("true,false") }}

apps/jetpack/templates/json/_edit_urls.json

@@ -19,6 +19,7 @@
     "remove_folder_url": "{{ revision.get_remove_folder_url() }}",
 	"all_modules_list_url": "{{ revision.get_modules_list_url() }}",
 	"conflicting_modules_list_url": "{{ revision.get_conflicting_modules_list_url() }}",
+  "revisions_list_html_url": "{{ url('jp_revisions_list_html', revision.pk) }}",
 	{% if revision.package.is_addon() %}
         "test_url": "{{ revision.get_test_xpi_url() }}",
         "download_url": "{{ revision.get_download_xpi_url() }}",

apps/jetpack/tests/attachment_test.py

@@ -181,21 +181,23 @@ class TestViews(TestCase):
         self.revision = next_revision(self.revision)
         return self.revision
 
-    def get_upload_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_upload_attachment', args=args)
+    def get_upload_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_package_revision_upload_attachment',
+                args=[revision.pk])
 
     def get_add_url(self, revision):
         args = [revision.pk]
         return reverse('jp_revision_add_attachment', args=args)
 
-    def get_change_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_save', args=args)
+    def get_change_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_revision_save', args=[revision.pk])
 
-    def get_delete_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_remove_attachment', args=args)
+    def get_delete_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_package_revision_remove_attachment',
+                args=[revision.pk])
 
     def get_revision(self):
         return PackageRevision.objects.get(pk=self.revision.pk)

apps/jetpack/tests/folders_tests.py

@@ -145,13 +145,13 @@ class TestViews(TestCase):
         self.revision = newest(self.revision)
         return self.revision
 
-    def get_add_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_add_folder', args=args)
+    def get_add_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_package_revision_add_folder', args=[revision.pk])
 
-    def get_delete_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_remove_folder', args=args)
+    def get_delete_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_package_revision_remove_folder', args=[revision.pk])
 
     def test_add_folder(self):
         res = self.post(self.get_add_url(self.revision.revision_number),

apps/jetpack/tests/module_tests.py

@@ -65,17 +65,17 @@ class TestModules(TestCase):
         self.revision = next(self.revision)
         return self.revision
 
-    def get_add_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_add_module', args=args)
+    def get_add_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_package_revision_add_module', args=[revision.pk])
 
-    def get_rename_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_rename_module', args=args)
+    def get_rename_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_package_revision_rename_module', args=[revision.pk])
 
-    def get_delete_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_remove_module', args=args)
+    def get_delete_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_addon_revision_remove_module', args=[revision.pk])
 
     def test_module_add(self):
         revision = self.add_one('a-module')

apps/jetpack/tests/test_views.py

@@ -66,7 +66,7 @@ class TestPackage(TestCase):
         assert 'save_url' in response.content
         # after setting the addon to private
         response = self.client.get(reverse('jp_package_disable',
-            args=[addon.id_number]))
+            args=[addon.pk]))
         self.client.login(username=user.username, password='secure')
         response = self.client.get(addon.get_absolute_url())
         assert 'save_url' in response.content
@@ -84,8 +84,7 @@ class TestPackage(TestCase):
         # logging in the author
         self.client.login(username=author.username, password='secure')
         # deleting lib
-        response = self.client.get(reverse('jp_package_delete',
-            args=[lib.id_number]))
+        response = self.client.get(reverse('jp_package_delete', args=[lib.pk]))
         eq_(response.status_code, 200)
         response = self.client.get(lib.get_absolute_url())
         # lib deleted - shouldn't be visible by author
@@ -108,7 +107,7 @@ class TestPackage(TestCase):
         self.client.login(username=author.username, password='secure')
         # private on
         response = self.client.get(reverse('jp_package_disable',
-            args=[lib.id_number]))
+            args=[lib.pk]))
         eq_(response.status_code, 200)
         response = self.client.get(lib.get_absolute_url())
         # lib private - should be visible by author
@@ -133,7 +132,7 @@ class TestPackage(TestCase):
         self.client.login(username=author.username, password='secure')
         # private on
         response = self.client.get(reverse('jp_package_disable',
-            args=[lib.id_number]))
+            args=[lib.pk]))
         eq_(response.status_code, 200)
         # logging in the user
         self.client.login(username=user.username, password='secure')
@@ -150,7 +149,7 @@ class TestPackage(TestCase):
         addon = Package.objects.create(
                 full_name='Public Add-on', author=user, type='a')
         response = self.client.get(reverse('jp_revisions_list_html',
-            args=[addon.id_number,]))
+            args=[addon.latest.pk,]))
         eq_(response.status_code, 200)
 
         # Private add-on
@@ -158,14 +157,42 @@ class TestPackage(TestCase):
                 full_name='Priv Add-on', author=user, type='a', active=False)
         # not logged in
         response = self.client.get(reverse('jp_revisions_list_html',
-            args=[addon.id_number,]))
+            args=[addon.latest.pk,]))
         eq_(response.status_code, 404)
         # authenticated
         self.client.login(username=user.username, password='secure')
         response = self.client.get(reverse('jp_revisions_list_html',
-            args=[addon.id_number,]))
+            args=[addon.latest.pk,]))
         eq_(response.status_code, 200)
 
+    def test_urls(self):
+        user = User.objects.get(username='jan')
+        addon = Package.objects.create(author=user, type='a')
+        revision = addon.latest
+        log.debug(revision.get_absolute_url())
+        eq_(revision.get_absolute_url(),
+                '/package/%d/' % revision.package.pk)
+        revision.save()
+        eq_(revision.get_absolute_url(),
+                '/package/%d/revision/%d/' % (revision.package.pk,
+                                              revision.revision_number))
+        revision.set_version('test')
+        version = PackageRevision.objects.get(pk=revision.pk)
+        version_pk = version.pk
+        eq_(revision.get_absolute_url(),
+                '/package/%d/' % revision.package.pk)
+        revision.save()
+        eq_(version.pk, version_pk)
+        eq_(revision.get_absolute_url(),
+                '/package/%d/revision/%s/' % (revision.package.pk,
+                                              revision.revision_number))
+        revision.set_version('test2')
+        eq_(revision.get_absolute_url(),
+                '/package/%d/' % revision.package.pk)
+        eq_(version.get_absolute_url(),
+                '/package/%d/version/%s/' % (version.package.pk,
+                                             version.version_name))
+
 
 class TestEmptyDirs(TestCase):
     fixtures = ['mozilla_user', 'users', 'core_sdk', 'packages']
@@ -192,13 +219,13 @@ class TestEmptyDirs(TestCase):
         self.revision = next_revision(self.revision)
         return self.revision
 
-    def get_add_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_add_folder', args=args)
+    def get_add_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_package_revision_add_folder', args=[revision.pk])
 
-    def get_delete_url(self, revision):
-        args = [self.package.id_number, revision]
-        return reverse('jp_addon_revision_remove_folder', args=args)
+    def get_delete_url(self, revision_number):
+        revision = self.package.revisions.get(revision_number=revision_number)
+        return reverse('jp_package_revision_remove_folder', args=[revision.pk])
 
     def test_add_folder(self):
         res = self.post(self.get_add_url(self.revision.revision_number),
@@ -254,7 +281,7 @@ class TestEditing(TestCase):
         rev = addon.latest
         rev.module_add(mod)
         r = self.client.get(
-                reverse('jp_revisions_list_html', args=[addon.id_number]))
+                reverse('jp_revisions_list_html', args=[addon.latest.pk]))
         assert 'test_filename' in r.content
 
     def test_package_name_change(self):
@@ -262,6 +289,7 @@ class TestEditing(TestCase):
         addon1 = Package(author=author, type='a')
         addon1.save()
         rev1 = addon1.latest
+        log.debug(addon1.latest.get_save_url())
         response = self.client.post(addon1.latest.get_save_url(), {
             'full_name': 'FULL NAME'})
         eq_(response.status_code, 200)
@@ -349,6 +377,7 @@ class TestRevision(TestCase):
         author.set_password('secure')
         author.save()
         self.client.login(username=author.username, password='secure')
+        log.debug(addon.latest.get_copy_url())
         response = self.client.get(addon.latest.get_copy_url())
         eq_(response.status_code, 200)
         assert 'Add-on' in response.content

apps/jetpack/urls.py

@@ -38,64 +38,38 @@ urlpatterns = patterns('jetpack.views',
     url(r'^library/new/',
         'create', {"type_id": "l"}, name='jp_library_create'),
 
-
-    # display details of the PackageRevision
-    url(r'^addon/(?P<id_number>[-\w]+)/latest/$',
-        'view_or_edit', {'type_id': 'a', 'latest': True},
-        name='jp_addon_latest'),
-    url(r'^library/(?P<id_number>[-\w]+)/latest/$',
-        'view_or_edit', {'type_id': 'l', 'latest': True},
-        name='jp_library_latest'),
-    url(r'^addon/(?P<id_number>[-\w]+)/$',
-        'view_or_edit', {'type_id': 'a'}, name='jp_addon_details'),
-    url(r'^library/(?P<id_number>[-\w]+)/$',
-        'view_or_edit', {'type_id': 'l'},  name='jp_library_details'),
-    url(r'^addon/(?P<id_number>[-\w]+)/version/(?P<version_name>.*)/$',
-        'view_or_edit', {'type_id': 'a'},
-        name='jp_addon_version_details'),
-    url(r'^library/(?P<id_number>[-\w]+)/version/(?P<version_name>.*)/$',
-        'view_or_edit', {'type_id': 'l'},
-        name='jp_library_version_details'),
-    url(r'^addon/(?P<id_number>[-\w]+)/revision/(?P<revision_number>\d+)/$',
-        'view_or_edit', {'type_id': 'a'},
-        name='jp_addon_revision_details'),
-    url(r'^library/(?P<id_number>[-\w]+)/revision/(?P<revision_number>\d+)/$',
-        'view_or_edit', {'type_id': 'l'},
-        name='jp_library_revision_details'),
+    # package - display details of the PackageRevision
+    url(r'^package/(?P<pk>\d+)/latest/$',
+        'view_or_edit', {'latest': True}, name='jp_latest'),
+    url(r'^package/(?P<pk>\d+)/$', 'view_or_edit', name='jp_details'),
+    url(r'^package/(?P<pk>\d+)/version/(?P<version_name>.*)/$',
+        'view_or_edit', name='jp_version_details'),
+    url(r'^package/(?P<pk>\d+)/revision/(?P<revision_number>\d+)/$',
+        'view_or_edit', name='jp_revision_details'),
 
     # get full module info
-    url(r'^get_module/(?P<id_number>[-\w]+)/(?P<revision_number>\d+)/'
-            '(?P<filename>.*)$', 'get_module', name='jp_get_module'),
+    url(r'^get_module/(?P<revision_id>\d+)/(?P<filename>.*)$',
+        'get_module', name='jp_get_module'),
     url(r'^module/(?P<pk>\d+)/$', 'download_module', name='jp_module'),
 
     # copy a PackageRevision
-    url(r'^addon/copy/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'copy', {'type_id': 'a'}, name='jp_addon_revision_copy'),
-    url(r'^library/copy/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'copy', {'type_id': 'l'},  name='jp_library_revision_copy'),
+    url(r'^package/copy/(?P<revision_id>\d+)/$',
+        'copy', name='jp_package_revision_copy'),
 
     # get Package revisions list
-    url(r'^revisions_list/(?P<id_number>[-\w]+)$',
-        'get_revisions_list_html', name='jp_revisions_list_html'),
-    url(r'^revisions_list/(?P<id_number>[-\w]+)/(?P<revision_number>\d+)/$',
+    url(r'^revisions_list/(?P<revision_id>\d+)/$',
         'get_revisions_list_html', name='jp_revisions_list_html'),
 
     # save packagerevision
-    url(r'^addon/save/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'save', {'type_id': 'a'}, name='jp_addon_revision_save'),
-    url(r'^library/save/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'save', {'type_id': 'l'},  name='jp_library_revision_save'),
+    url(r'^package/save/(?P<revision_id>\d+)/$',
+        'save', name='jp_revision_save'),
 
     # disable/activate/delete package
-    url(r'^package/disable/(?P<id_number>[-\w]+)/$',
+    url(r'^package/disable/(?P<pk>[-\w]+)/$',
         'disable', name='jp_package_disable'),
-    url(r'^package/activate/(?P<id_number>[-\w]+)/$',
+    url(r'^package/activate/(?P<pk>[-\w]+)/$',
         'activate', name='jp_package_activate'),
-    url(r'^package/delete/(?P<id_number>[-\w]+)/$',
+    url(r'^package/delete/(?P<pk>[-\w]+)/$',
         'delete', name='jp_package_delete'),
 
     # get all, conflicting modules
@@ -106,91 +80,38 @@ urlpatterns = patterns('jetpack.views',
         name='jp_revision_get_conflicting_modules_list'),
 
     # add/remove module
-    url(r'^addon/add_module/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'add_module',
-        {'type_id': 'a'}, name='jp_addon_revision_add_module'),
-    url(r'^library/add_module/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'add_module',
-        {'type_id': 'l'},  name='jp_library_revision_add_module'),
-    url(r'^addon/remove_module/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'remove_module',
-        {'type_id': 'a'}, name='jp_addon_revision_remove_module'),
-    url(r'^library/remove_module/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'remove_module',
-        {'type_id': 'l'},  name='jp_library_revision_remove_module'),
+    url(r'^package/add_module/(?P<revision_id>\d+)/$',
+        'add_module', name='jp_package_revision_add_module'),
+    url(r'^package/remove_module/(?P<revision_id>\d+)/$',
+        'remove_module', name='jp_package_revision_remove_module'),
 
     # rename module
-    url(r'^addon/rename_module/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'rename_module',
-        {'type_id': 'a'}, name='jp_addon_revision_rename_module'),
-    url(r'^library/rename_module/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'rename_module',
-        {'type_id': 'l'}, name='jp_library_revision_rename_module'),
+    url(r'^package/rename_module/(?P<revision_id>\d+)/$',
+        'rename_module', name='jp_package_revision_rename_module'),
 
     # switch SDK version
-    url(r'^addon/switch_sdk/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
+    url(r'^package/switch_sdk/(?P<revision_id>\d+)/$',
         'switch_sdk', name='jp_addon_switch_sdk_version'),
 
     # add/remove attachment
-    url(r'^addon/upload_attachment/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'upload_attachment',
-        {'type_id': 'a'}, name='jp_addon_revision_upload_attachment'),
-    url(r'^library/upload_attachment/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'upload_attachment',
-        {'type_id': 'l'},  name='jp_library_revision_upload_attachment'),
+    url(r'^package/upload_attachment/(?P<revision_id>\d+)/$',
+        'upload_attachment', name='jp_package_revision_upload_attachment'),
     url(r'^revision/(?P<pk>\d+)/add_attachment/',
         'revision_add_attachment', name='jp_revision_add_attachment'),
-    url(r'^addon/remove_attachment/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'remove_attachment',
-        {'type_id': 'a'}, name='jp_addon_revision_remove_attachment'),
-    url(r'^library/remove_attachment/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'remove_attachment',
-        {'type_id': 'l'},  name='jp_library_revision_remove_attachment'),
+    url(r'^package/remove_attachment/(?P<revision_id>\d+)/$',
+        'remove_attachment', name='jp_package_revision_remove_attachment'),
 
      # rename attachment
-    url(r'^addon/rename_attachment/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'rename_attachment',
-        {'type_id': 'a'}, name='jp_addon_revision_rename_attachment'),
-    url(r'^library/rename_attachment/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'rename_attachment',
-        {'type_id': 'l'}, name='jp_library_revision_rename_attachment'),
+    url(r'^package/rename_attachment/(?P<revision_id>\d+)/$',
+        'rename_attachment', name='jp_package_revision_rename_attachment'),
 
     #add empty dir
-    url(r'^addon/add_folder/(?P<id_number>[-\w]+)/revision'
-            '(?P<revision_number>\d+)/$',
-        'add_folder',
-        {'type_id': 'a'}, name='jp_addon_revision_add_folder',
-        ),
-    url(r'^library/add_folder/(?P<id_number>[-\w]+)/revision'
-            '(?P<revision_number>\d+)/$',
-        'add_folder',
-        {'type_id': 'l'}, name='jp_library_revision_add_folder',
-        ),
+    url(r'^package/add_folder/(?P<revision_id>\d+)/$',
+        'add_folder', name='jp_package_revision_add_folder'),
 
     #remove empty dir
-    url(r'^addon/remove_folder/(?P<id_number>[-\w]+)/revision'
-            '(?P<revision_number>\d+)/$',
-        'remove_folder',
-        {'type_id': 'a'}, name='jp_addon_revision_remove_folder',
-        ),
-    url(r'^library/remove_folder/(?P<id_number>[-\w]+)/revision'
-            '(?P<revision_number>\d+)/$',
-        'remove_folder',
-        {'type_id': 'l'}, name='jp_library_revision_remove_folder',
-        ),
+    url(r'^package/remove_folder/(?P<revision_id>\d+)/$',
+        'remove_folder', name='jp_package_revision_remove_folder'),
 
     # display attachment
     url(r'^attachment/(?P<uid>.*)$',
@@ -200,43 +121,18 @@ urlpatterns = patterns('jetpack.views',
     url(r'^autocomplete/library/$',
         'library_autocomplete', name='jp_library_autocomplete'),
     # assign library
-    url(r'^addon/assign_library/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'assign_library',
-        {'type_id': 'a'}, name='jp_addon_revision_assign_library'),
-    url(r'^library/assign_library/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'assign_library',
-        {'type_id': 'l'}, name='jp_library_revision_assign_library'),
+    url(r'^package/assign_library/(?P<revision_id>\d+)/$',
+        'assign_library', name='jp_package_revision_assign_library'),
 
     # update library
-    url(r'^addon/update_library/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'update_library',
-        {'type_id': 'a'}, name='jp_addon_revision_update_library'),
-    url(r'^library/update_library/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'update_library',
-        {'type_id': 'l'}, name='jp_library_revision_update_library'),
+    url(r'^package/update_library/(?P<revision_id>\d+)/$',
+        'update_library', name='jp_package_revision_update_library'),
 
     # remove library
-    url(r'^addon/remove_dependency/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'remove_library',
-        {'type_id': 'a'}, name='jp_addon_revision_remove_library'),
-    url(r'^library/remove_dependency/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'remove_library',
-        {'type_id': 'l'},  name='jp_library_revision_remove_library'),
+    url(r'^package/remove_dependency/(?P<revision_id>\d+)/$',
+        'remove_library', name='jp_package_revision_remove_library'),
 
     # check libraries for latest versions
-    url(r'addon/check_latest_dependencies/(?P<id_number>[-\w]+)/revision/'
-            '(?P<revision_number>\d+)/$',
-        'latest_dependencies',
-        {'type_id': 'a'}, name='jp_addon_check_latest_dependencies'),
-
-    url(r'library/check_latest_dependencies/(?P<id_number>[-\w]+)/revision'
-            '(?P<revision_number>\d+)/$',
-        'latest_dependencies',
-        {'type_id': 'l'}, name='jp_library_check_latest_dependencies'),
+    url(r'package/check_latest_dependencies/(?P<revision_id>\d+)/$',
+        'latest_dependencies', name='jp_package_check_latest_dependencies'),
 )

apps/jetpack/views.py

@@ -92,13 +92,14 @@ def browser(request, page_number=1, type_id=None, username=None):
         })
 
 
-def view_or_edit(request, id_number, type_id, revision_number=None,
-                         version_name=None, latest=False):
+def view_or_edit(request, pk=None, id_number=None, type_id=None,
+        revision_number=None, version_name=None, latest=False):
     """
     Edit if user is the author, otherwise view
     """
-    revision = get_package_revision(id_number, type_id,
+    revision = get_package_revision(pk, id_number, type_id,
                                     revision_number, version_name, latest)
+
     edit_available = True
     if revision.package.deleted:
         edit_available = False
@@ -201,14 +202,12 @@ def download_module(request, pk):
     return HttpResponse(module.get_json())
 
 
-def get_module(request, id_number, revision_number, filename):
+def get_module(request, revision_id, filename):
     """
     return a JSON with all module info
     """
     try:
-        revision = PackageRevision.objects.get(
-                package__id_number=id_number,
-                revision_number=revision_number)
+        revision = PackageRevision.objects.get(pk=revision_id)
         mod = revision.modules.get(filename=filename)
     except PackageRevision.DoesNotExist, Module.DoesNotExist:
         log_msg = 'No such module %s' % filename
@@ -225,21 +224,18 @@ def get_module(request, id_number, revision_number, filename):
 
 @transaction.commit_on_success
 @login_required
-def copy(request, id_number, type_id,
-                 revision_number=None, version_name=None):
+def copy(request, revision_id):
     """
     Copy package - create a duplicate of the Package, set user as author
     """
-    source = get_package_revision(id_number, type_id, revision_number,
-                                  version_name)
-    pk = source.pk
-    log.debug('[copy: %s] Copying started from (%s)' % (pk, source))
+    source = get_object_with_related_or_404(PackageRevision, pk=revision_id)
+    log.debug('[copy: %s] Copying started from (%s)' % (revision_id, source))
 
     # save package
     try:
         package = source.package.copy(request.user)
     except IntegrityError, err:
-        log.critical(("[copy: %s] Package copy failed") % pk)
+        log.critical(("[copy: %s] Package copy failed") % revision_id)
         return HttpResponseForbidden('You already have a %s with that name' %
                                      escape(source.package.get_type_name()))
 
@@ -248,21 +244,22 @@ def copy(request, id_number, type_id,
     copied = source
     del source
 
-    log.info('[copy: %s] Copied to %s, (%s)' % (pk, copied.pk, copied.full_name))
+    log.info('[copy: %s] Copied to %s, (%s)' % (revision_id, copied.pk,
+                                                copied.full_name))
     return render_json(request,
         "json/%s_copied.json" % package.get_type_name(),
         {'revision': copied})
 
 
 @login_required
-def disable(request, id_number):
+def disable(request, pk):
     """
     Disable Package and return confirmation
     """
-    package = get_object_or_404(Package, id_number=id_number)
+    package = get_object_or_404(Package, pk=pk)
     if request.user.pk != package.author.pk:
         log_msg = 'User %s wanted to disable not his own Package %s.' % (
-            request.user, id_number)
+            request.user, pk)
         log.warning(log_msg)
         return HttpResponseForbidden(
             'You are not the author of this %s' % escape(
@@ -276,14 +273,14 @@ def disable(request, id_number):
 
 
 @login_required
-def activate(request, id_number):
+def activate(request, pk):
     """
     Undelete Package and return confirmation
     """
-    package = get_object_or_404(Package, id_number=id_number)
+    package = get_object_or_404(Package, pk=pk)
     if request.user.pk != package.author.pk:
         log_msg = ("[security] Attempt to activate package (%s) by "
-                   "non-owner (%s)" % (id_number, request.user))
+                   "non-owner (%s)" % (pk, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden(
             'You are not the author of this %s' % escape(
@@ -297,14 +294,14 @@ def activate(request, id_number):
 
 
 @login_required
-def delete(request, id_number):
+def delete(request, pk):
     """
     Delete Package and return confirmation
     """
-    package = get_object_or_404(Package, id_number=id_number)
+    package = get_object_or_404(Package, pk=pk)
     if request.user.pk != package.author.pk:
         log_msg = ("[security] Attempt to delete package (%s) by "
-                   "non-owner (%s)" % (id_number, request.user))
+                   "non-owner (%s)" % (pk, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden(
             'You are not the author of this %s' % escape(
@@ -317,13 +314,11 @@ def delete(request, id_number):
 
 @require_POST
 @login_required
-def add_module(request, id_number, type_id, revision_number=None,
-        version_name=None):
+def add_module(request, revision_id):
     """
     Add new module to the PackageRevision
     """
-    revision = get_package_revision(id_number, type_id, revision_number,
-                                    version_name)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
         log_msg = ("[security] Attempt to add a module to package (%s) by "
                    "non-owner (%s)" % (id_number, request.user))
@@ -355,14 +350,14 @@ def add_module(request, id_number, type_id, revision_number=None,
 
 @require_POST
 @login_required
-def rename_module(request, id_number, type_id, revision_number):
+def rename_module(request, revision_id):
     """
     Rename a module in a PackageRevision
     """
-    revision = get_package_revision(id_number, type_id, revision_number)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
         log_msg = ("[security] Attempt to rename a module to package (%s) by "
-                   "non-owner (%s)" % (id_number, request.user))
+                   "non-owner (%s)" % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden('You are not the author of this Package')
 
@@ -390,7 +385,7 @@ def rename_module(request, id_number, type_id, revision_number):
 
     if not module:
         log_msg = 'Attempt to rename a non existing module %s from %s.' % (
-            old_name, id_number)
+            old_name, revision_id)
         log.warning(log_msg)
         return HttpResponseForbidden(
             'There is no such module in %s' % escape(
@@ -407,14 +402,14 @@ def rename_module(request, id_number, type_id, revision_number):
 
 @require_POST
 @login_required
-def remove_module(request, id_number, type_id, revision_number):
+def remove_module(request, revision_id):
     """
     Remove module from PackageRevision
     """
-    revision = get_package_revision(id_number, type_id, revision_number)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
         log_msg = ("[security] Attempt to remove a module from package (%s) "
-                "by non-owner (%s)" % (id_number, request.user))
+                "by non-owner (%s)" % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden('You are not the author of this Package')
 
@@ -426,7 +421,7 @@ def remove_module(request, id_number, type_id, revision_number):
                 filenames)
     except Module.DoesNotExist:
         log_msg = 'Attempt to delete a non existing module(s) %s from %s.' % (
-            str(filenames), id_number)
+            str(filenames), revision_id)
         log.warning(log_msg)
         return HttpResponseForbidden(
             'There is no such module in %s' % escape(
@@ -441,12 +436,12 @@ def remove_module(request, id_number, type_id, revision_number):
 
 @require_POST
 @login_required
-def add_folder(request, id_number, type_id, revision_number):
+def add_folder(request, revision_id):
     " adds an EmptyDir to a revision "
-    revision = get_package_revision(id_number, type_id, revision_number)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
-        log_msg = ("[security] Attempt to add a folder to package (%s) by "
-                   "non-owner (%s)" % (id_number, request.user))
+        log_msg = ("[security] Attempt to add a folder to revision (%s) by "
+                   "non-owner (%s)" % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden('You are not the author of this Package')
 
@@ -469,12 +464,12 @@ def add_folder(request, id_number, type_id, revision_number):
 
 @require_POST
 @login_required
-def remove_folder(request, id_number, type_id, revision_number):
+def remove_folder(request, revision_id):
     " removes an EmptyDir from a revision "
-    revision = get_package_revision(id_number, type_id, revision_number)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
-        log_msg = ("[security] Attempt to remove a folder from package (%s) "
-                "by non-owner (%s)" % (id_number, request.user))
+        log_msg = ("[security] Attempt to remove a folder from revision (%s) "
+                "by non-owner (%s)" % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden('You are not the author of this Package')
 
@@ -489,7 +484,7 @@ def remove_folder(request, id_number, type_id, revision_number):
             response = revision.attachment_rmdir(foldername)
         if not response:
             log_msg = 'Attempt to delete a non existing folder %s from %s.' % (
-                foldername, id_number)
+                foldername, revision_id)
             log.warning(log_msg)
             return HttpResponseForbidden(
                 'There is no such folder in %s' % escape(
@@ -511,9 +506,9 @@ def remove_folder(request, id_number, type_id, revision_number):
 
 @require_POST
 @login_required
-def switch_sdk(request, id_number, revision_number):
+def switch_sdk(request, revision_id):
     " switch SDK used to create XPI - sdk_id from POST "
-    revision = get_package_revision(id_number, 'a', revision_number)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
         return HttpResponseForbidden('You are not the author of this Add-on')
 
@@ -535,15 +530,14 @@ def switch_sdk(request, id_number, revision_number):
 
 @require_POST
 @login_required
-def upload_attachment(request, id_number, type_id,
-                           revision_number=None, version_name=None):
+def upload_attachment(request, revision_id):
     """ Upload new attachment to the PackageRevision
     """
-    revision = get_package_revision(id_number, type_id, revision_number,
-                                    version_name)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
+    log.debug(revision)
     if request.user.pk != revision.author.pk:
         log_msg = ("[security] Attempt to upload attachment to package (%s) "
-                "by non-owner (%s)" % (id_number, request.user))
+                "by non-owner (%s)" % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden(
             'You are not the author of this %s' % escape(
@@ -553,8 +547,8 @@ def upload_attachment(request, id_number, type_id,
     filename = request.META.get('HTTP_X_FILE_NAME')
 
     if not f:
-        log_msg = 'Path not found: %s, package: %s.' % (
-            filename, id_number)
+        log_msg = 'Path not found: %s, revision: %s.' % (
+            filename, revision_id)
         log.error(log_msg)
         return HttpResponseServerError('Path not found.')
 
@@ -585,7 +579,7 @@ def upload_attachments(request, id_number, type_id,
                            revision_number=None, version_name=None):
     """ Upload new attachments to the PackageRevision
     """
-    revision = get_package_revision(id_number, type_id, revision_number,
+    revision = get_package_revision(None, id_number, type_id, revision_number,
                                     version_name)
     if request.user.pk != revision.author.pk:
         log_msg = ("[security] Attempt to upload attachment to package (%s) "
@@ -624,7 +618,7 @@ def add_empty_attachment(request, id_number, type_id,
                            revision_number=None, version_name=None):
     """ Add new empty attachment to the PackageRevision
     """
-    revision = get_package_revision(id_number, type_id, revision_number,
+    revision = get_package_revision(None, id_number, type_id, revision_number,
                                     version_name)
     if request.user.pk != revision.author.pk:
         log_msg = ("[security] Attempt to add attachment to package (%s) by "
@@ -742,14 +736,14 @@ def revision_add_attachment(request, pk):
 @require_POST
 @login_required
 @transaction.commit_on_success
-def rename_attachment(request, id_number, type_id, revision_number):
+def rename_attachment(request, revision_id):
     """
     Rename an attachment in a PackageRevision
     """
-    revision = get_package_revision(id_number, type_id, revision_number)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
-        log_msg = ("[security] Attempt to rename attachment in package (%s) "
-                "by non-owner (%s)" % (id_number, request.user))
+        log_msg = ("[security] Attempt to rename attachment in revision (%s) "
+                "by non-owner (%s)" % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden('You are not the author of this Package')
 
@@ -758,7 +752,7 @@ def rename_attachment(request, id_number, type_id, revision_number):
         attachment = revision.attachments.get(pk=uid)
     except:
         log_msg = ('Attempt to rename a non existing attachment. attachment: '
-                   '%s, package: %s.' % (uid, id_number))
+                   '%s, revision: %s.' % (uid, revision))
         log.warning(log_msg)
         return HttpResponseForbidden(
             'There is no such attachment in %s' % escape(
@@ -805,14 +799,14 @@ def rmdir(request, pk, target, path):
 
 @require_POST
 @login_required
-def remove_attachment(request, id_number, type_id, revision_number):
+def remove_attachment(request, revision_id):
     """
     Remove attachment from PackageRevision
     """
-    revision = get_package_revision(id_number, type_id, revision_number)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
-        log_msg = ("[security] Attempt to remove attachment from package (%s) "
-                "by non-owner (%s)" % (id_number, request.user))
+        log_msg = ('[security] Attempt to remove attachment from revision '
+                '(%s) by non-owner (%s)' % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden('You are not the author of this Package')
 
@@ -822,7 +816,7 @@ def remove_attachment(request, id_number, type_id, revision_number):
 
     if not attachment:
         log_msg = ('Attempt to remove a non existing attachment. attachment: '
-                   '%s, package: %s.' % (uid, id_number))
+                   '%s, revision: %s.' % (uid, revision_id))
         log.warning(log_msg)
         return HttpResponseForbidden(
             'There is no such attachment in %s' % escape(
@@ -855,18 +849,17 @@ def download_attachment(request, uid):
 
 @require_POST
 @login_required
-def save(request, id_number, type_id, revision_number=None,
-                 version_name=None):
+def save(request, revision_id, type_id=None):
     """
     Save package and modules
     @TODO: check how dynamic module loading affects save
     """
 
-    revision = get_package_revision(id_number, type_id, revision_number,
-                                    version_name)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
         log_msg = ("[security] Attempt to save package (%s) by "
-                   "non-owner (%s)" % (id_number, request.user))
+                   "non-owner (%s)" % (revision.pk,
+                       request.user))
         log.warning(log_msg)
         return HttpResponseForbidden('You are not the author of this Package')
 
@@ -881,13 +874,12 @@ def save(request, id_number, type_id, revision_number=None,
     jid = request.POST.get('jid', None)
     version_name = request.POST.get('version_name', False)
 
-    # validate package_full_name and version_name
-
     if jid and not validator.is_valid(
         'alphanum_plus', jid):
         return HttpResponseForbidden(escape(
             validator.get_validation_message('alphanum_plus')))
 
+    # validate package_full_name and version_name
     if version_name and not validator.is_valid(
         'alphanum_plus', version_name):
         return HttpResponseForbidden(escape(
@@ -1016,8 +1008,7 @@ def create(request, type_id):
 
     item.save()
 
-    return HttpResponseRedirect(reverse(
-        'jp_%s_latest' % item.get_type_name(), args=[item.id_number]))
+    return HttpResponseRedirect(reverse('jp_latest', args=[item.pk]))
 
 
 @require_POST
@@ -1081,17 +1072,16 @@ def library_autocomplete(request):
 
 @require_POST
 @login_required
-def assign_library(request, id_number, type_id,
-                           revision_number=None, version_name=None):
+def assign_library(request, revision_id):
     " assign library to the package "
-    revision = get_package_revision(id_number, type_id, revision_number,
-                                    version_name)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
-        log_msg = ("[security] Attempt to assign library to package (%s) by "
-                   "non-owner (%s)" % (id_number, request.user))
+        log_msg = ("[security] Attempt to assign library to revision (%s) by "
+                   "non-owner (%s)" % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden('You are not the author of this Package')
 
+    # TODO: make linking work with library_id instead of id number
     library = get_object_or_404(
         Package, type='l', id_number=request.POST['id_number'])
     if request.POST.get('use_latest_version', False):
@@ -1118,17 +1108,18 @@ def assign_library(request, id_number, type_id,
 
 @require_POST
 @login_required
-def remove_library(request, id_number, type_id, revision_number):
+def remove_library(request, revision_id):
     " remove dependency from the library provided via POST "
-    revision = get_package_revision(id_number, type_id, revision_number)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
-        log_msg = ("[security] Attempt to remove library from package (%s) by "
-                   "non-owner (%s)" % (id_number, request.user))
+        log_msg = ("[security] Attempt to remove library from revision (%s) by "
+                   "non-owner (%s)" % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden(
             'You are not the author of this %s' % escape(
                 revision.package.get_type_name()))
 
+    # TODO: make unlinking work with library_id instead of id number
     lib_id_number = request.POST.get('id_number')
     library = get_object_or_404(Package, id_number=lib_id_number)
 
@@ -1144,17 +1135,18 @@ def remove_library(request, id_number, type_id, revision_number):
 
 @require_POST
 @login_required
-def update_library(request, id_number, type_id, revision_number):
+def update_library(request, revision_id):
     " update a dependency to a certain version "
-    revision = get_package_revision(id_number, type_id, revision_number)
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     if request.user.pk != revision.author.pk:
-        log_msg = ("[security] Attempt to update library in package (%s) by "
-                   "non-owner (%s)" % (id_number, request.user))
+        log_msg = ("[security] Attempt to update library in revision (%s) by "
+                   "non-owner (%s)" % (revision_id, request.user))
         log.warning(log_msg)
         return HttpResponseForbidden(
             'You are not the author of this %s' % escape(
                 revision.package.get_type_name()))
 
+    # TODO: make updating work with library_id instead of id number
     lib_id_number = request.POST.get('id_number')
     lib_revision = request.POST.get('revision')
 
@@ -1174,8 +1166,8 @@ def update_library(request, id_number, type_id, revision_number):
 
 
 @login_required
-def latest_dependencies(request, id_number, type_id, revision_number):
-    revision = get_package_revision(id_number, type_id, revision_number)
+def latest_dependencies(request, revision_id):
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
     out_of_date = revision.get_outdated_dependency_versions()
 
     return render_json(request,
@@ -1183,21 +1175,16 @@ def latest_dependencies(request, id_number, type_id, revision_number):
 
 
 @never_cache
-def get_revisions_list_html(request, id_number, revision_number=None):
+def get_revisions_list_html(request, revision_id):
     " returns revision list to be displayed in the modal window "
-    package = get_object_with_related_or_404(Package, id_number=id_number)
-    if not package.can_view(request.user):
+    current = get_object_with_related_or_404(PackageRevision, pk=revision_id)
+    if not current.package.can_view(request.user):
         raise Http404
-    revisions = package.revisions.all()
-    if revision_number:
-        current = package.revisions.get(revision_number=revision_number)
-    else:
-        current = None
-    if revision_number:
-        revision_number = int(revision_number)
+    revisions = current.package.revisions.all()
+    revision_number = int(current.revision_number)
     return render(request,
         '_package_revisions_list.html', {
-            'package': package,
+            'package': current.package,
             'revisions': revisions,
             'revision_number': revision_number,
             'current': current})

apps/xpi/tests/test_building.py

@@ -203,8 +203,7 @@ class XPIBuildTest(TestCase):
             author=self.author,
             type='l'
         )
-        librev = PackageRevision.objects.filter(
-            package__id_number=lib.id_number)[0]
+        librev = lib.latest
         self.addonrev.dependency_add(librev)
         tstart = time.time()
         xpi_utils.sdk_copy(self.addonrev.sdk.get_source_dir(), self.SDKDIR)

apps/xpi/tests/test_views.py

@@ -30,7 +30,7 @@ class TestViews(TestCase):
         self.check_download_url = reverse('jp_check_download_xpi',
                 args=[self.hashtag])
         self.prepare_test_url = reverse('jp_addon_revision_test',
-                args=['1000003', 0])
+                args=[205])
         self.xpi_path = os.path.join(
             settings.XPI_TARGETDIR, '%s.xpi' % self.hashtag)
 
@@ -72,14 +72,13 @@ class TestViews(TestCase):
     def test_hashtag(self):
         revision = PackageRevision.objects.get(pk=205)
         uri = reverse('jp_addon_revision_test',
-            args=[revision.package.id_number, revision.revision_number])
+            args=[revision.pk])
         response = self.client.post(uri, {'hashtag': 'abc/123'})
         eq_(response.status_code, 400)
         response = self.client.post(uri, {'hashtag': self.hashtag})
         eq_(response.status_code, 200)
         response = self.client.post(
-                reverse('jp_addon_revision_xpi', args=[
-                    revision.package.id_number, revision.revision_number]),
+                reverse('jp_addon_revision_xpi', args=[revision.pk]),
                 {'hashtag': 'abc.123'})
         eq_(response.status_code, 400)
         response = self.client.get('/xpi/test/abc/123')
@@ -92,7 +91,7 @@ class TestViews(TestCase):
         addon = Package.objects.create(author=user, type='a',
                 active=False)
         prepare_test_url = reverse('jp_addon_revision_test',
-                args=[addon.id_number, addon.latest.revision_number])
+                args=[addon.latest.pk])
         # test unauthenticated
         response = self.client.post(prepare_test_url, {
             'hashtag': 'abc'})
@@ -118,7 +117,7 @@ class TestViews(TestCase):
         addon = Package.objects.create(author=user, type='a',
                 active=False)
         prepare_download_url = reverse('jp_addon_revision_xpi',
-                args=[addon.id_number, addon.latest.revision_number])
+                args=[addon.latest.pk])
         # test unauthenticated
         response = self.client.post(prepare_download_url, {
             'hashtag': 'abc'})

apps/xpi/urls.py

@@ -3,12 +3,10 @@
 from django.conf.urls.defaults import url, patterns
 
 urlpatterns = patterns('xpi.views',
-    # test Add-on's PackageRevision
-    url(r'^prepare_test/(?P<id_number>[-\w]+)/revision/'
-        '(?P<revision_number>\d+)/$',
+    # create XPI
+    url(r'^prepare_test/(?P<revision_id>\d+)/$',
         'prepare_test', name='jp_addon_revision_test'),
-    url(r'^prepare_download/(?P<id_number>[-\w]+)/revision/'
-        '(?P<revision_number>\d+)/$',
+    url(r'^prepare_download/(?P<revision_id>\d+)/$',
         'prepare_download', name='jp_addon_revision_xpi'),
 
     # get and remove created XPI

apps/xpi/views.py

@@ -25,21 +25,21 @@ from xpi import tasks
 log = commonware.log.getLogger('f.xpi')
 
 
-def _get_addon(user, id_number, revision_number):
-    revision = get_object_with_related_or_404(PackageRevision,
-                        package__id_number=id_number, package__type='a',
-                        revision_number=revision_number)
-    if not revision.package.active and user != revision.package.author:
+def _get_addon(user, revision_id):
+    revision = get_object_with_related_or_404(PackageRevision, pk=revision_id)
+    if ((not revision.package.active and user != revision.package.author)
+            or revision.package.type != 'a'):
+        # pretend add-on doesn't exist as it's a Library or is private
         raise Http404()
     return revision
 
 @csrf_exempt
 @require_POST
-def prepare_test(r, id_number, revision_number=None):
+def prepare_test(r, revision_id):
     """
     Test XPI from data saved in the database
     """
-    revision = _get_addon(r.user, id_number, revision_number)
+    revision = _get_addon(r.user, revision_id)
     hashtag = r.POST.get('hashtag')
     if not hashtag:
         log.warning('[security] No hashtag provided')
@@ -124,12 +124,12 @@ def get_test(r, hashtag):
 
 @csrf_exempt
 @require_POST
-def prepare_download(r, id_number, revision_number=None):
+def prepare_download(r, revision_id):
     """
     Prepare download XPI.  This package is built asynchronously and we assume
     it works. It will be downloaded in %``get_download``
     """
-    revision = _get_addon(r.user, id_number, revision_number)
+    revision = _get_addon(r.user, revision_id)
     hashtag = r.POST.get('hashtag')
     if not hashtag:
         return HttpResponseForbidden('Add-on Builder has been updated!'