Skip to content

Conversation

@SwampFalc
Copy link

So in #175 it was mentioned that, ideally, the whole process of creating the RDN should be separate from designating primary keys for the database table.

@Natureshadow mentioned they were not keen on such a major rewrite, though.

But looking at the code, I believe there's quite an easy path, namely a simple class attribute, for which I propose rdn_fields, taking a list of field names. The code for build_rdn is trivial to change.

As far as the user is concerned:

  1. They are already expected to add some class attributes. Requiring one more should not be strange or surprising.
  2. Previously, if they did not add a primary_key field, they got an error when saving. Now, if they do not add the class attribute, they get the exact same error at the exact same time.
  3. Creating a list of field names is a common occurrence in Django: for Meta ordering, for the admin site, for a ModelForm, etc.

In short, as far as user experience goes, I do not see this as a major difference.

Is it backwards compatible? Well, not as I wrote it, no. But you can easily turn the if slightly more complicated to achieve that:
if field.db_column and (field.primary_key or field.name in self.rdn_fields):

@Natureshadow
Copy link
Member

@Natureshadow mentioned they were not keen on such a major rewrite, though.

Where did I say that?

In any case, please make this change backwards-compatible.

@SwampFalc
Copy link
Author

I was referring to this comment: #175 (comment)

@Natureshadow
Copy link
Member

I was referring to this comment: #175 (comment)

I can't remember what the reason for this comment was. That said, I will test and review the change in the course of the week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants