BryanDavis has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/370134 )

Change subject: Change ToolInfo authors to list of strings
......................................................................


Change ToolInfo authors to list of strings

Using tool maintainers/LDAP users as the author list was a bad idea, so
lets undo it. Tools can have origins from lots of places and we should
not assume that everything will be credited to active local users.

The django migrations for this may make a mess of things. I ended up
needing to manually change the foreign key constraints in the
tools_toolinfo_authors table. This should not be a problem in
a production environment where the tables are managed with hand curated
SQL statements. If it turns out to be problematic for developers we may
need to hack up the migration files in a followup.

Bug: T149458
Change-Id: I586148a9dcfd3ef7d6b1f09e64b890a695dc4e8e
---
M striker/tools/forms.py
A striker/tools/migrations/0009_toolinfo_authors.py
M striker/tools/models.py
M striker/tools/urls.py
M striker/tools/views/tool.py
M striker/tools/views/toolinfo.py
6 files changed, 89 insertions(+), 20 deletions(-)

Approvals:
  BryanDavis: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/striker/tools/forms.py b/striker/tools/forms.py
index 809e3e7..47ec66a 100644
--- a/striker/tools/forms.py
+++ b/striker/tools/forms.py
@@ -172,6 +172,9 @@
             'tags': autocomplete.ModelSelect2Multiple(
                 url='tools:tags_autocomplete',
             ),
+            'authors': autocomplete.ModelSelect2Multiple(
+                url='tools:api:author',
+            ),
             'repository': forms.TextInput(
                 attrs={
                     'placeholder': _("URL to your tool's source code"),
diff --git a/striker/tools/migrations/0009_toolinfo_authors.py 
b/striker/tools/migrations/0009_toolinfo_authors.py
new file mode 100644
index 0000000..d86307c
--- /dev/null
+++ b/striker/tools/migrations/0009_toolinfo_authors.py
@@ -0,0 +1,29 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('tools', '0008_ldap_models'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Author',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', auto_created=True, 
serialize=False, primary_key=True)),
+                ('name', models.CharField(max_length=128, unique=True)),
+            ],
+            options={
+                'ordering': ('name',),
+            },
+        ),
+        migrations.AlterField(
+            model_name='toolinfo',
+            name='authors',
+            field=models.ManyToManyField(related_name='_toolinfo_authors_+', 
to='tools.Author'),
+        ),
+    ]
diff --git a/striker/tools/models.py b/striker/tools/models.py
index 9384b12..84823dd 100644
--- a/striker/tools/models.py
+++ b/striker/tools/models.py
@@ -255,6 +255,17 @@
         ordering = ('name',)
 
 
+class Author(models.Model):
+    """Describe an author."""
+    name = models.CharField(max_length=128, unique=True)
+
+    def __str__(self):
+        return self.name
+
+    class Meta:
+        ordering = ('name',)
+
+
 @reversion.register()
 class ToolInfo(models.Model):
     """Metadata about a Tool hosted on Toolforge.
@@ -266,8 +277,7 @@
     title = models.CharField(max_length=255)
     description = models.TextField()
     license = models.ForeignKey(SoftwareLicense)
-    authors = models.ManyToManyField(
-        settings.AUTH_USER_MODEL, related_name='+')
+    authors = models.ManyToManyField(Author, related_name='+')
     tags = models.ManyToManyField(ToolInfoTag, blank=True)
     repository = models.CharField(max_length=2047, blank=True, null=True)
     issues = models.CharField(max_length=2047, blank=True, null=True)
@@ -297,7 +307,6 @@
             ('description', self.description),
             ('url', url),
             ('keywords', ", ".join(tag.name for tag in self.tags.all())),
-            ('author', ", ".join(
-                a.get_full_name() for a in self.authors.all())),
+            ('author', ", ".join(a.name for a in self.authors.all())),
             ('repository', self.repository),
         ])
diff --git a/striker/tools/urls.py b/striker/tools/urls.py
index e6378a4..bda603a 100644
--- a/striker/tools/urls.py
+++ b/striker/tools/urls.py
@@ -20,10 +20,11 @@
 
 from django.conf import urls
 
-from striker.tools.views.toolinfo import HistoryView
-from striker.tools.views.toolinfo import TagAutocomplete
 from striker.tools.views.tool import MaintainerAutocomplete
 from striker.tools.views.tool import ToolUserAutocomplete
+from striker.tools.views.toolinfo import AuthorAutocomplete
+from striker.tools.views.toolinfo import HistoryView
+from striker.tools.views.toolinfo import TagAutocomplete
 
 
 TOOL = r'(?P<tool>[_a-z][-0-9_a-z]*)'
@@ -134,6 +135,10 @@
         urls.patterns(
             'striker.tools.views',
             urls.url(
+                r'autocomplete/author',
+                AuthorAutocomplete.as_view(),
+                name='author'),
+            urls.url(
                 r'^autocomplete/maintainer$',
                 MaintainerAutocomplete.as_view(),
                 name='maintainer'),
@@ -143,7 +148,7 @@
                 name='tooluser'),
             urls.url(
                 r'^toolname/(?P<name>.+)$',
-                'striker.tools.views.tool.name_available',
+                'tool.toolname_available',
                 name='toolname'),
         ),
         namespace='api'
diff --git a/striker/tools/views/tool.py b/striker/tools/views/tool.py
index 2149a39..4565f36 100644
--- a/striker/tools/views/tool.py
+++ b/striker/tools/views/tool.py
@@ -39,6 +39,7 @@
 from striker.tools import utils
 from striker.tools.forms import MantainersForm
 from striker.tools.forms import ToolCreateForm
+from striker.tools.models import Author
 from striker.tools.models import DiffusionRepo
 from striker.tools.models import Maintainer
 from striker.tools.models import ToolInfo
@@ -92,7 +93,9 @@
                         reversion.set_user(req.user)
                         reversion.set_comment('Tool created')
                         toolinfo.save()
-                        toolinfo.authors.add(req.user)
+                        founder, created = Author.objects.get_or_create(
+                            name=req.user.get_full_name())
+                        toolinfo.authors.add(founder)
                         if form.cleaned_data['tags']:
                             toolinfo.tags.add(*form.cleaned_data['tags'])
                 except Exception:
diff --git a/striker/tools/views/toolinfo.py b/striker/tools/views/toolinfo.py
index 0fc0523..375c68f 100644
--- a/striker/tools/views/toolinfo.py
+++ b/striker/tools/views/toolinfo.py
@@ -38,6 +38,7 @@
 
 from striker.tools.forms import ToolInfoForm
 from striker.tools.forms import ToolInfoPublicForm
+from striker.tools.models import Author
 from striker.tools.models import Tool
 from striker.tools.models import ToolInfo
 from striker.tools.models import ToolInfoTag
@@ -48,7 +49,6 @@
 logger = logging.getLogger(__name__)
 
 
-@reversion.views.create_revision()
 @login_required
 @inject_tool
 def create(req, tool):
@@ -69,19 +69,20 @@
     if req.method == 'POST':
         if form.is_valid():
             try:
-                reversion.set_comment(form.cleaned_data['comment'])
-                toolinfo = form.save(commit=False)
-                toolinfo.tool = tool.name
-                toolinfo.save()
-                form.save_m2m()
-                reversion.add_to_revision(toolinfo)
+                with reversion.create_revision():
+                    reversion.set_comment(form.cleaned_data['comment'])
+                    toolinfo = form.save(commit=False)
+                    toolinfo.tool = tool.name
+                    toolinfo.save()
+                    form.save_m2m()
+                    reversion.add_to_revision(toolinfo)
                 messages.info(
                     req, _("Toolinfo {} created".format(toolinfo.title)))
                 return shortcuts.redirect(
                     urlresolvers.reverse('tools:tool', kwargs={
                         'tool': tool.name,
                     }))
-            except DatabaseError:
+            except DatabaseError as dbe:
                 logger.exception('ToolInfo.save failed')
                 messages.error(
                     req,
@@ -105,7 +106,6 @@
     return shortcuts.render(req, 'tools/info/read.html', ctx)
 
 
-@reversion.views.create_revision()
 @login_required
 @inject_tool
 def edit(req, tool, info_id):
@@ -121,9 +121,10 @@
     if req.method == 'POST':
         if form.is_valid():
             try:
-                reversion.set_comment(form.cleaned_data['comment'])
-                toolinfo = form.save()
-                reversion.add_to_revision(toolinfo)
+                with reversion.create_revision():
+                    reversion.set_comment(form.cleaned_data['comment'])
+                    toolinfo = form.save()
+                    reversion.add_to_revision(toolinfo)
                 messages.info(
                     req, _("Toolinfo {} updated".format(toolinfo.title)))
                 return shortcuts.redirect(
@@ -303,6 +304,25 @@
         return ToolInfoTag.objects.create(name=text, slug=slugify(text))
 
 
+class AuthorAutocomplete(autocomplete.Select2QuerySetView):
+    create_field = 'name'
+
+    def get_queryset(self):
+        if not self.request.user.is_authenticated():
+            return Author.objects.none()
+        qs = Author.objects.all()
+        if self.q:
+            qs = qs.filter(name__icontains=self.q)
+        qs.order_by('name')
+        return qs
+
+    def has_add_permission(self, request):
+        return request.user.is_authenticated()
+
+    def create_object(self, text):
+        return Author.objects.create(name=text)
+
+
 def json_v1(req):
     class PrettyPrintJSONEncoder(DjangoJSONEncoder):
         def __init__(self, *args, **kwargs):

-- 
To view, visit https://gerrit.wikimedia.org/r/370134
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I586148a9dcfd3ef7d6b1f09e64b890a695dc4e8e
Gerrit-PatchSet: 3
Gerrit-Project: labs/striker
Gerrit-Branch: master
Gerrit-Owner: BryanDavis <bda...@wikimedia.org>
Gerrit-Reviewer: BryanDavis <bda...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to