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