Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

local-flavor NL: add model fields, renaming, testing, deprecating. #150

Closed
2 of 3 tasks
jieter opened this issue Apr 15, 2015 · 10 comments
Closed
2 of 3 tasks

local-flavor NL: add model fields, renaming, testing, deprecating. #150

jieter opened this issue Apr 15, 2015 · 10 comments

Comments

@jieter
Copy link
Contributor

jieter commented Apr 15, 2015

As mentioned in this PR comment I'm about to do some work on the Dutch local-flavor fields. While working on reorganizing and adding the model fields and tests, a couple of questions rise:

  • Since everyone is now required to use IBAN, there is no use for NLBankAccountNumber* anymore. I suggest removing(or at least deprecating) it.
  • The term 'Sofinummer' is succeeded in common use by the term 'Burger Service Nummer', I suggest renaming SoFi to BSN. (There is an edge case)
  • It seems to be common to call model fields FooField and form fields FooFormField. This is not pattern used for the current fields. Should we rename the form fields?

cc @benkonrath

@benkonrath
Copy link
Member

Point 1: Some users may actually still want to store the old Dutch bank numbers but I don't know for sure. I suppose the question is really about whether localflavor should keep historical fields or not. If we were to remove the NLBankAccountNumber* fields, they would need to be depreciated first and I think it would be better to file a separate issue to solve this generally for all the SEPA bank fields in localflavor.

Point 2 and Point 3: You would need to provide South and native Django migrations for anything you want to rename. Although I don't know off hand what the localflavor policy for renaming fields is.

@jezdez @erikr @claudep Are any of you guys able to provide more information here? Thanks.

@mxsasha
Copy link
Member

mxsasha commented May 2, 2015

On point 1: I think we should deprecate all local bank account fields, in countries where IBAN has become mandatory. By mandatory I mean that in the Netherlands, I can no longer enter an old form account number in a bank transfer, and the conversion service has shut down. So there is no possible use for an old form account number anymore. I believe this is part of SEPA, and therefore required in all SEPA countries. For non-SEPA IBAN countries we should then keep the existing field. Also, special territories of some countries are excluded from SEPA, like Greenland or Sint Maarten, so if they still use the Danish/Dutch old formats, we need to keep the fields of that country.

Point 2: I'm not sure whether wikipedia is (still) correct regarding the edge case: it seems there is (now) a special registration for niet-ingezetenen which does get you a BSN. I do think we should rename the field, but with a deprecation step for the old name. I'm not sure how this fits in with migrations (perhaps it'll just work).

Point 3: I like consistency, but I don't think the benefits outweigh the inconvenience for all the users. So I'm not inclined to rename them.

@jieter
Copy link
Contributor Author

jieter commented May 3, 2015

@benkonrath @erikr Thanks for your comments on this. I'll look into it later this week and propose a change we can discuss after that.

@benkonrath
Copy link
Member

@jieter Is it safe to assume that you closed this issue because you're not planning to work on this any more? I'm planning to setup a deprecation policy for localflavor. Would you be interested in doing the SofiNumber -> BSN rename in the Dutch localflavor after my work has landed?

@jieter jieter reopened this Dec 2, 2015
@jieter
Copy link
Contributor Author

jieter commented Dec 2, 2015

Hmm, I was to quick in closing this. Might be better to create a new issue of the rename though, I'm willing to work on it then

@benkonrath
Copy link
Member

@jieter If you're still interested in working on this, now would be a good time. There are a few examples of deprecations in the code base that you can use to help you get started. Just search for RemovedInLocalflavor20Warning.

@jieter
Copy link
Contributor Author

jieter commented Jul 27, 2017

I can invest some time in about two weeks.

@jieter
Copy link
Contributor Author

jieter commented Jul 28, 2017

#307.

Regarding the comment of @erikr about special terratories of The Netherlands: I don't think that applies as they have a different country code, so it doesn't make sense to save it in a NLBankAccountNumberField.

For example, Sint Maarten

@benkonrath
Copy link
Member

@jieter I deprecated the SoFiNumber fields and added BSN fields in #314. Are you still interested in renaming the form fields? If not, this issue can be closed. Renaming the NL form fields would need to happen in the django-localflavor 2.x release cycle.

@jieter
Copy link
Contributor Author

jieter commented Nov 23, 2017

Hmm, I think it's not worth the effort, lets keep them like they are...

@jieter jieter closed this as completed Nov 23, 2017
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

No branches or pull requests

3 participants