Ability to manually edit concordances from the Location edit page

[simonw]

Since I disabled the ability to edit the vaccinefinder_location_id and suchlike fields.

See discussion https://discord.com/channels/799147121357881364/813861006718926848/834600191461687378

[simonw]

One thing that worries me about this is that concordances aren't covered by the django-reversion mechanism, and adding them to it is non-trivial (classic definition here: non-trivial = "I don't know how to do that").

For the moment I'm going to go ahead without adding support for version tracking, but I feel like we're going to want that.

One possibility: denormalize these to be a JSON array on the Location, which can then be revision tracked by our existing system.

[simonw]

For the moment though I'm going to use Django edit inlines.

[simonw]

Hmm... all of the options seem bad.

class LocationConcordanceInline(admin.TabularInline):
    model = Location.concordances.through
    extra = 2
    autocomplete_fields = ("concordanceidentifier",)

class LocationConcordanceInline(admin.TabularInline):
    model = Location.concordances.through
    extra = 2
    raw_id_fields = ("concordanceidentifier",)

[simonw]

What I really want here is two boxes - for authority and identifier - which the user can fill in directly.

[simonw]

Maybe I could instead customize the form displayed by the admin to show fields for a fixed number of authorities, then have the admin save use those to populate the concordances.

[obra]

In terms of not having django-reversion: can we make sure that we're logging the heck out of this?

On Wed, Apr 21, 2021 at 8:47 PM Simon Willison @.***> wrote:

Maybe I could instead customize the form displayed by the admin to show fields for a fixed number of authorities, then have the admin save use those to populate the concordances.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CAVaccineInventory/vial/issues/391#issuecomment-824516239, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAALC2BDWANPRDD5KZBWT5LTJ6L4BANCNFSM43LOG6MA .

[simonw]

I tried using a custom form for that admin class:

class LocationAdminForm(forms.ModelForm):
    concordance_google_places = forms.CharField(required=False)
    concordance_vaccinefinder = forms.CharField(required=False)
    concordance_vaccinespotter = forms.CharField(required=False)

    def save(self, commit=True):
        return super().save(commit=commit)

    class Meta:
        model = Location
        exclude = [] #  Weirdly this is required

@admin.register(Location)
class LocationAdmin(DynamicListDisplayMixin, CompareVersionAdmin):
    form = LocationAdminForm
    # ...
    fieldsets = (
        # ...
        ("Identifiers", {"fields": (
            "public_id",
            "concordances_summary",
            "concordance_google_places",
            "concordance_vaccinefinder",
            "concordance_vaccinespotter",
        )}),

But I couldn't figure out a clean pattern for populating those new custom form fields with their current values.

[simonw]

I think the ugly raw_id_fields thing is the best short-term solution here.

[simonw]

If I can change the copy here I can at least make it a bit less confusing!

[simonw]

These help a bit:

class LocationConcordanceInline(admin.StackedInline):
    model = Location.concordances.through
    verbose_name = "Identifier"
    verbose_name_plural = "Identifiers in other systems"
    extra = 2
    raw_id_fields = ("concordanceidentifier",)

[simonw]

Hah, turns out what I want here might even be a 15 year old open Django feature request! https://code.djangoproject.com/ticket/897

[simonw]

Linked from that ticket, I dug this up from the Internet Archive: https://web.archive.org/web/20110723065646/https://snipt.net/chrisdpratt/symmetrical-manytomany-filter-horizontal-in-django-admin/

[simonw]

I'm going to try using a custom template for the inline, see if I can make the thing a bit more usable that way.

Here are the defaults: https://github.com/django/django/blob/ca9872905559026af82000e46cde6f7dedc897b6/django/contrib/admin/options.py#L2199-L2204

class StackedInline(InlineModelAdmin):
    template = 'admin/edit_inline/stacked.html'

class TabularInline(InlineModelAdmin):
    template = 'admin/edit_inline/tabular.html'

https://github.com/django/django/blob/4a77aeb1f86bc06e18023cac10109e067ed20800/django/contrib/admin/templates/admin/edit_inline/stacked.html

[simonw]

class LocationConcordanceInline(admin.StackedInline):
    model = Location.concordances.through
    verbose_name = "Identifier"
    verbose_name_plural = "Identifiers in other systems"
    extra = 2
    raw_id_fields = ("concordanceidentifier",)
    template = "admin/location_concordance_inline.html"

Then in admin/location_concordance_inline.html:

<h2>How to use identifiers in other systems</h2>
<p>Instructions go here.</p>
{% include "admin/edit_inline/stacked.html" %}

Output:

[simonw]

I'd really like to customize that ConcordanceIdentifier_locations object (795) string. There's a nasty technique to do that here: https://github.com/jazzband/django-sortedm2m/issues/96#issue-206260561

def sortedm2m__str__(self):
    """
    Improves string representation of m2m relationship objects
    """
    return self.config.name

Config.templates.through.__str__ = sortedm2m__str__

[simonw]

I did this:

ConcordanceIdentifier.locations.through.__str__ = lambda self: str(self.concordanceidentifier)

And got this:

[simonw]

No, this interface is just horrible for this purpose. Custom templates aren't going to make it better.

I'm going to use JavaScript to add a custom interface to this area of the Django Admin that allows identifiers to be assigned in a more usable way. This interface will only show on edit, not on initial add - I think that's OK.

[simonw]

Additional bonus: I can use the API logs (at least for the moment) to track who did what.

[simonw]

Missing API feature: the ability to authenticate API calls using Django cookies, as opposed to API keys.

[simonw]

I've spent far too long on this. I've bodged together just enough jQuery to provide a solution and I'm calling it done.

[simonw]

[simonw]

The logging is working: here's a page showing all API logs for that endpoint with an associated user ID: https://vial.calltheshots.us/admin/api/apilog/?path=%2Fapi%2FupdateLocationConcordances&user__isnull=0