Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#22588 closed Bug (fixed)

makemigrations and regexvalidators

Reported by: leftmoose Owned by: nobody
Component: Core (Other) Version: 1.7-beta-2
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

comparing RegexValidators doesn't quite work as expected, since it compares compiled instances

not sure why manage.py makemigrations still works (maybe re cache?), but the issue arises if e.g. using call_command(makemigrations)

cat app/models.py
from django.core import validators
from django.db import models


# Create your models here.
class MyModel(models.Model):
    field = models.CharField(
        max_length=1,
        validators=[validators.RegexValidator('[A-Za-z]')],
    )

(single shell session, no changes in the background)

In [1]: from django.core.management import call_command

In [2]: call_command('makemigrations')
Migrations for 'app':
  0001_initial.py:
    - Create model MyModel

In [3]: call_command('makemigrations')
Migrations for 'app':
  0002_auto_20140506_2031.py:
    - Alter field field on mymodel

In [4]: call_command('makemigrations')
Migrations for 'app':
  0003_auto_20140506_2032.py:
    - Alter field field on mymodel

In [5]: call_command('makemigrations')
Migrations for 'app':
  0004_auto_20140506_2032.py:
    - Alter field field on mymodel

Change History (9)

comment:1 Changed 2 years ago by leftmoose

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 2 years ago by Tim Graham

Patch needs improvement: set
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:3 Changed 2 years ago by Andrew Godwin

Resolution: worksforme
Status: newclosed

This is a duplicate of the issue in #21638, and I've verified that on master it works as expected (I suspect, leftmoose, you're using the beta, where this wasn't fixed yet). Closing as WORKSFORME.

comment:4 Changed 2 years ago by leftmoose

Resolution: worksforme
Status: closednew

@andrewgodwin, if you look at my patch, it sits on top of the fix from #21638. this is not a duplicate. a68f32579145dfbd51d87e14f9b5e04c770f9081 compares regex pattern objects, which i change to comparing their parameters. since a68f325 the regex validator has also grown an inverse_match argument, which is ignored. finally, you added __eq__, but no __ne__ which means assertNotEquals don't work properly. please see the pr above

comment:5 Changed 2 years ago by Claude Paroz

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:6 Changed 2 years ago by David Szotten <davidszotten@…>

Resolution: fixed
Status: newclosed

In 724a7bf222e31f42e2ab431cc09da224081d03fa:

[1.7.x] Fixed #22588 -- Fix RegexValidator eq

Compare parameters instead of re.pattern instances, and add the other
parameters to the comparison. Also add a ne to make assertNotEqual
work properly.

comment:7 Changed 2 years ago by Andrew Godwin <andrew@…>

In d2e96b5792367af5aa0140f7e2673d654bb266df:

Merge pull request #2637 from davidszotten/validator_comparisons

[1.7.x] Fixed #22588 -- Fix RegexValidator eq

comment:8 Changed 2 years ago by Andrew Godwin <andrew@…>

In 7fe60ae64ad712430bce26af7812ed4452a91af0:

Fixed #22588 -- Fix RegexValidator eq

Compare parameters instead of re.pattern instances, and add the other
parameters to the comparison. Also add a ne to make assertNotEqual
work properly.

comment:9 Changed 2 years ago by Andrew Godwin

Sorry, my bad - we've had the "validators don't compare properly" reopened multiple times recently with people using the beta rather than master. A more descriptive title might have helped, though :)

I've merged in the PR and forward-ported it to master. Let me know if you see anything else wrong.

Note: See TracTickets for help on using tickets.
Back to Top