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

How to add extra address line for Apt/Unit#? #141

Closed
xjlin0 opened this issue Dec 31, 2020 · 3 comments
Closed

How to add extra address line for Apt/Unit#? #141

xjlin0 opened this issue Dec 31, 2020 · 3 comments
Labels
In-Progress This ticket is in active development
Milestone

Comments

@xjlin0
Copy link
Contributor

xjlin0 commented Dec 31, 2020

Great package! Would it be possible to add one extra field of Address to enter extra info such as apartment/floor/unit info of the same street address? Or should we add extra info in the route field instead? Here is my PR, please let me know if it's ok to add one such field, thanks!
#142

class Address(models.Model):
    extra = models.CharField(max_length=20, blank=True)

Also , since ordering will be ('locality', 'route', 'street_number', 'extra'), probably it's better to add index to route/street_number/extra for the db performance.

@banagale banagale added the In-Progress This ticket is in active development label Dec 31, 2020
@banagale banagale added this to the Version 0.2.6 milestone Dec 31, 2020
@banagale
Copy link
Collaborator

Hello Jack, thank you for the praise and the PR.

I understand there could be use cases where having an apartment number/floor/unit specific field on the Address model would be useful.

However, I am reluctant to merge an arbitrary new extra field into the model at this stage to handle this.

The reason is the project is installed in many places and there is already a goal to handle a better, more abstract model architecture.

I will look again at this as I look at other more basic improvements but probably would not accept this field.

Instead, I would suggest you use the Django Address model, but subclass it and add your extra field. This will allow you to continue to take updates as they may come in.

I did see your search error fix for admin. I haven't validated this but I can merge that commit once I do.

@xjlin0
Copy link
Contributor Author

xjlin0 commented Jan 1, 2021

Sure no problems! thanks again for the suggestion, Merry Christmas and Happy New Year!

@xjlin0 xjlin0 closed this as completed Jan 1, 2021
@banagale
Copy link
Collaborator

banagale commented Jan 2, 2021

Sure no problems! thanks again for the suggestion, Merry Christmas and Happy New Year!

You too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In-Progress This ticket is in active development
Projects
None yet
Development

No branches or pull requests

2 participants