[PATCH] Add help text to project and patch_project fields

[Veronika Kabatova]

----- Original Message -----
> From: "Stephen Finucane" <stephen at that.guru>
> To: "Daniel Axtens" <dja at axtens.net>, vkabatov at redhat.com, patchwork at lists.ozlabs.org
> Sent: Sunday, September 30, 2018 12:23:13 AM
> Subject: Re: [PATCH] Add help text to project and patch_project fields
> 
> On Sat, 2018-09-29 at 01:24 +1000, Daniel Axtens wrote:
> > vkabatov at redhat.com writes:
> > 
> > > From: Veronika Kabatova <vkabatov at redhat.com>
> > > 
> > > Duplication of the field to avoid performance issues caused some
> > > unexpected results when moving patches between projects in the admin
> > > interface. It's easy to change the "project" field and click save,
> > > instead of double checking which field needs to be modified and kept in
> > > sync with the one being changed. This lead to patch showing correct
> > > project in the API and patch detail in the web UI, but the patch itself
> > > was listed in the wrong project when looking at the patch listings.
> > > 
> > > Because of the discussions of the denormalization of the tables, trying
> > > to code automatic modification of the other field if one is being
> > > changed seems like too much work for very little benefit. What we can do
> > > instead is mention this dependency in fields' help text. Modification of
> > > the project is not an everyday task so it shouldn't put too much burden
> > > on the users and this simple reminder can prevent unexpected breakage
> > > and bug reports.
> > > 
> > 
> > Acked-by: Daniel Axtens <dja at axtens.net>
> > > Signed-off-by: Veronika Kabatova <vkabatov at redhat.com>
> > 
> > Stephen, I think this should go to stable/2.1. Thoughts?
> 
> I don't think it can. I haven't checked yet but I'm pretty sure this
> requires a migration and we can't backport those.
> 
> Rather than doing this, could we not just override the 'save' function
> to always save the other field?
> 

We'd need to figure out which one of the fields is the changed one and
modify the other one to match it, which based on [1] doesn't look as easy
as it sounds. "I'll scroll till I find the field I want" and "here's the
long content, I'll go to the end and scroll up" would lead to the same but
opposite issue instead of fixing the problem, if we decided to hardcode one
of the fields as the one to be copied over.

Because of the above, I opted for the simplest help text solution. I'm not
opposed to implement a more complicated solution that doesn't require any
steps from the user (it was the first solution I looked into) if you think
the complications are worth it, or have a better idea how to work around it.


Veronika


[1] https://stackoverflow.com/questions/1355150/django-when-saving-how-can-you-check-if-a-field-has-changed

> Stephen
> 
> > Regards,
> > Daniel
> > 
> > > ---
> > >  patchwork/models.py | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/patchwork/models.py b/patchwork/models.py
> > > index a043844d..e30a9739 100644
> > > --- a/patchwork/models.py
> > > +++ b/patchwork/models.py
> > > @@ -350,7 +350,11 @@ class FilenameMixin(object):
> > >  class Submission(FilenameMixin, EmailMixin, models.Model):
> > >      # parent
> > >  
> > > -    project = models.ForeignKey(Project, on_delete=models.CASCADE)
> > > +    project = models.ForeignKey(Project, on_delete=models.CASCADE,
> > > +                                help_text='If modifying this field on
> > > Patch, '
> > > +                                'don\'t forget to modify the "Patch
> > > project" '
> > > +                                'field appropriately as well to ensure
> > > proper '
> > > +                                'functionality!')
> > >  
> > >      # submission metadata
> > >  
> > > @@ -405,7 +409,11 @@ class Patch(Submission):
> > >  
> > >      # duplicate project from submission in subclass so we can count the
> > >      # patches in a project without needing to do a JOIN.
> > > -    patch_project = models.ForeignKey(Project, on_delete=models.CASCADE)
> > > +    patch_project = models.ForeignKey(Project, on_delete=models.CASCADE,
> > > +                                      help_text='"Project" field needs
> > > to be '
> > > +                                      'kept in sync and changed manually
> > > in '
> > > +                                      'case of modifications to ensure
> > > proper '
> > > +                                      'functionality!')
> > >  
> > >      objects = PatchManager()
> > >  
> > > --
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > Patchwork mailing list
> > > Patchwork at lists.ozlabs.org
> > > https://lists.ozlabs.org/listinfo/patchwork
> > 
> > _______________________________________________
> > Patchwork mailing list
> > Patchwork at lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork
> 
> 
>