Code

Opened 6 years ago

Closed 6 years ago

#5819 closed (duplicate)

FlatPage model field url is not unique

Reported by: jesper@… Owned by: nobody
Component: Contrib apps Version: master
Severity: Keywords: flatpages
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

You can create several pages with the same URL, which makes the middleware barf.

I'm not submitting a patch, but I'd imagine it to be as easy as adding an index on (url, site_id).

Attachments (1)

5819.diff (775 bytes) - added by arien <regexbot@…> 6 years ago.
trivial patch to ensure url is unique

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by arien <regexbot@…>

trivial patch to ensure url is unique

comment:1 Changed 6 years ago by arien <regexbot@…>

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by Jesper Noehr <jesper@…>

  • Patch needs improvement set

Arien,

I don't think it's an appropriate fix to set unique=True on url alone. Flatpages are per site, and sites can have the same urls. I think the unique constraint needs to be on (url, site_id) as I mentioned in my initial report.

comment:3 Changed 6 years ago by arien <regexbot@…>

Right you are; I can't believe I missed that both in your description and the docs...!

I don't think this can be fixed in a meaningful way without changing the many-to-many relationship between FlatPages and Sites. For example, what should happen if you create an /about/ page and decide to use it on more than one site, and then later you add create another /about/ page for one of these same sites (and possibly others)?

comment:4 Changed 6 years ago by Jesper Noehr <jesper@…>

Hm, you have a good point. Although--without knowing the inner workings on ManyToMany--I do believe that if the unique constraint lives on (url, site_id), then you *can* use it cross-site. E.g.

('/about/', 1) and ('/about/', 2) would be fine with that constraint. However, you cannot create a new ('/about', 2) anymore, since the contraint won't allow you to. Please correct me if I'm wrong :-)

comment:5 Changed 6 years ago by arien <regexbot@…>

  • Has patch unset
  • Patch needs improvement unset

You cannot have a unique constraint across tables.

You could prevent creating duplicates by checking what combinations are "taken" before saving: you raise an exception if the FlatPage you're about to save has one or more Sites that are already used by another FlatPage (i.e. one with a different id) with the same URL (path, really). But you'd have to do that in a transaction, and since not all databases used with Django are guaranteed to have those...

Changing the view to use get_list_or_404 (and picking one of the results) instead of get_object_or_404 isn't really an option either, so I think punting was probably the right decision.

comment:6 follow-up: Changed 6 years ago by Jesper Noehr <jesper@…>

I don't entirely agree with that. The constraint is not across tables, as I'm not suggesting (url, site) but (url, site_ID). Both columns are in the same table, so it's a simple unique constraint.

comment:7 in reply to: ↑ 6 Changed 6 years ago by ubernostrum

Replying to Jesper Noehr <jesper@noehr.org>:

I don't entirely agree with that. The constraint is not across tables, as I'm not suggesting (url, site) but (url, site_ID). Both columns are in the same table, so it's a simple unique constraint.

No, they are not in the same table; the url column is in the flatpages table, and the site_id column is in a separate join table used to implement the many-to-many relationship. Go look at your database if you don't believe me.

comment:8 Changed 6 years ago by anonymous

I believe you, thank you for clarifying. As I wrote earlier, I don't know how manytomany relationships work in Django, but of course they would be linked through a link-table, now that you mention it.

I'm not really sure what to do about the constraint then.

comment:9 Changed 6 years ago by arien <regexbot@…>

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

This is a duplicate of #702.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.