Opened 15 months ago

Closed 15 months ago

Last modified 15 months 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 15 months ago by leftmoose

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 15 months ago by timo

  • Patch needs improvement set
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

comment:3 Changed 15 months ago by andrewgodwin

  • Resolution set to worksforme
  • Status changed from new to closed

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 15 months ago by leftmoose

  • Resolution worksforme deleted
  • Status changed from closed to new

@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 15 months ago by claudep

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

comment:6 Changed 15 months ago by David Szotten <davidszotten@…>

  • Resolution set to fixed
  • Status changed from new to closed

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 15 months 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 15 months 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 15 months ago by andrewgodwin

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