Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  closed
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Sardorbek Imomaliev):

 * cc: Sardorbek Imomaliev (added)


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.0c8ff55f62d9ce01cae7f17cbf5cc4e5%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  closed
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Sardorbek Imomaliev):

 Hi Carlton,

 Thanks for reply, it is not broken because there is no `Not Empty` choice
 in django itself
 created new issue https://code.djangoproject.com/ticket/30149

 Replying to [comment:11 Carlton Gibson]:
 > Hi Sardorbek.
 >
 > This looks like a (related but) separate issue. Can you submit a new
 ticket, something along the lines of "Empty value `selected` check in
 Admin Filter prevents subclassing" and we can have a look.
 >
 > At first glance I can't see how both `bool(self.lookup_val_isnull)` and
 `self.lookup_val_isnull == 'False'` evaluate as `True` in the `Not Empty`
 case. (???) Surely the existing `empty_choice` logic would already be
 broken if they did.
 >
 > If you have a PR (with a test case showing the broken example) available
 (or can provide one when opening the ticket) it makes it much easier to
 assess.
 >
 > Thanks.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.a2339a1996e00097868c5486aa5b797f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  closed
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tim Graham):

 * resolution:  fixed => wontfix


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.1016bdaf2383aee2b999ecb95930c421%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  closed
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * status:  new => closed
 * resolution:   => fixed


Comment:

 Hi Sardorbek.

 This looks like a (related but) separate issue. Can you submit a new
 ticket, something along the lines of "Empty value `selected` check in
 Admin Filter prevents subclassing" and we can have a look.

 At first glance I can't see how both `bool(self.lookup_val_isnull)` and
 `self.lookup_val_isnull == 'False'` evaluate as `True` in the `Not Empty`
 case. (???) Surely the existing `empty_choice` logic would already be
 broken if they did.

 If you have a PR (with a test case showing the broken example) available
 (or can provide one when opening the ticket) it makes it much easier to
 assess.

 Thanks.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.840482ad51afcec325479b3b11d26174%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  new
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Sardorbek Imomaliev):

 * status:  closed => new
 * resolution:  wontfix =>


Comment:

 I think we should reopen this and fix an issue with current
 `RelatedFieldListFilter`. For example currently to add `Not Empty` choice
 you need to do this

 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python
   class NotEmptyFilter(admin.RelatedFieldListFilter):
   @property
   def include_empty_choice(self):
   # FIXME: empty value selected incorrectly override in choices
   return False

   def has_output(self):
   return super().has_output() + 2

   def choices(self, changelist):
   yield from super().choices(changelist)
   yield {
   'selected': self.lookup_val_isnull == 'True',
   'query_string': changelist.get_query_string(
   {self.lookup_kwarg_isnull: 'True'}, [self.lookup_kwarg]
   ),
   'display': _('Empty'),
   yield {
   'selected': self.lookup_val_isnull == 'False',
   'query_string': changelist.get_query_string(
   {self.lookup_kwarg_isnull: 'False'}, [self.lookup_kwarg]
   ),
   'display': _('Not Empty'),
   }
   }}}
 }}}

 But you should be able to do just
 {{{
 #!div style="font-size: 80%"
 Code highlighting:
   {{{#!python
   class NotEmptyFilter(admin.RelatedFieldListFilter):
   def has_output(self):
   return super().has_output() + 1

   def choices(self, changelist):
   yield from super().choices(changelist)
   yield {
   'selected': self.lookup_val_isnull == 'False',
   'query_string': changelist.get_query_string(
   {self.lookup_kwarg_isnull: 'False'}, [self.lookup_kwarg]
   ),
   'display': _('Not Empty'),
   }
   }}}
 }}}

 Currently this is not possible because of `bool(self.lookup_val_isnull)`
 check in selected for `Empty` choice, and if we add `Not Empty` choice
 like in second example we will get both `Empty` and `Not Empty` choices
 rendered as selected on `Not Empty`

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.d8834ce0eae1c6f61121aab9ccc0001b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  closed
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:  wontfix
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tim Graham):

 * status:  assigned => closed
 * resolution:   => wontfix


Comment:

 After some discussion on the pull request, I think it's better if users
 subclass the relevant list filter and add the "not empty" option if they
 desire it. It doesn't seem like a particularly common need and that way
 it's not cluttering things for everyone else.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.1a0844527cce27209c1b0559560545e4%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tim Graham):

 * needs_better_patch:  0 => 1
 * stage:  Ready for checkin => Accepted


Comment:

 I agree -- `get_not_empty_value_display()` looks heavy handed for this use
 case. `get_empty_value_display()` is used elsewhere besides the
 `RelatedFieldListFilter` and it corresponds to the values displayed in the
 changelist -- that's not the case for the "not empty" case.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.c49e2035060af56795e02e1b2b12a4e3%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * needs_better_patch:  1 => 0
 * stage:  Accepted => Ready for checkin


Comment:

 OK, patch looks good.

 I have a small reservation about whether we want yet more API on
 ModelAdmin to add the `not_empty_value_display` option, rather than
 steering users to subclassing `FieldListFilter` themselves, but bar
 that...

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.297bf2d2938f8c31d4aeb504b447aa75%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * needs_better_patch:  0 => 1


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.e1a7a8f55e433ec6ddaa5b0c8a714a7e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Claude Paroz):

 As for the display value, it should mimic the
 `model_admin.get_empty_value_display()` behavior
 (`get_not_empty_value_display()`).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.cea0adb281fca14cc1a02a237ec7377f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * needs_better_patch:  1 => 0


Comment:

 PR is functionally OK.

 It hardcodes a (translatable) "Not Empty" string. I'm not sure if we need
 to do something more sophisticated there (or take a different approach,
 such as using a filter subclass if you want this behaviour) or if it's OK
 as it is.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.3a2cc4dcc4b048954d65cfbe4547072d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Carlton Gibson):

 * needs_better_patch:  0 => 1
 * has_patch:  0 => 1


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.dbf173b8d402a6ecfb9cfe7c27af46dc%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #28687: Add a 'Not Empty' option to admin's related filter (was: Not 'Empty' Related Filter option)

[Django]

#28687: Add a 'Not Empty' option to admin's related filter
-+-
 Reporter:  Brillgen Developers  |Owner:  Jake
 Type:   |  Barber
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.admin|  Version:  2.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Tim Graham):

 * type:  Uncategorized => Cleanup/optimization
 * easy:  1 => 0
 * stage:  Unreviewed => Accepted


Comment:

 Seems okay at first glance.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.e88991e0832aa623e267261c8b0b209d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.