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

Improve referencing in the API for the wallet and shop #12

Open
flaeppe opened this issue Feb 8, 2017 · 3 comments
Open

Improve referencing in the API for the wallet and shop #12

flaeppe opened this issue Feb 8, 2017 · 3 comments
Assignees
Milestone

Comments

@flaeppe
Copy link
Member

flaeppe commented Feb 8, 2017

UUID referencing in the wallet and shop models should be replaced to be more generic yet still more identifying.

A single UUID reference does not, in some cases, achieve enough object identification, whereas if we mimic a generic foreign key relation we can instead identify an object with its type and UUID.

This can all be done without exposing any underlying model(for wallet and shop), hence we can better protect against eventual sidestepping of their API and unwanted usage/behaviour.

@kjagiello kjagiello added this to the 2.0.0 milestone Feb 8, 2017
@kjagiello
Copy link
Member

Checklist:

  • shop.models.ProductTransaction (fixed in ff88d07)
  • wallet.models.WalletTransaction

@flaeppe
Copy link
Member Author

flaeppe commented Apr 12, 2017

This issue is going to be complicated to resolve before any adjustments to the following field on the WalletTransaction is in place:

reference = models.CharField(max_length=128, blank=True, null=True)

As this field can be filled in with custom text (i.e. Swishinsättning), it would not be wise to enforce stronger generic referencing with ContentType, as sometimes the reference is not a database object.

However, this argument assumes that the reference for a transaction should always be an object stored somewhere in the database.

The question here would be:
Should we represent a reference like Swishinsättning differently? Is it enough with a CharField?

@kjagiello
Copy link
Member

IMO we should. In a couple of months, as soon Länsförsäkringar has released Swish för handel, we will be in need of storing more metadata about Swish payments and a CharField won't be enough. Feels like the most natural way of solving this is to store the metadata in a separate table.

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

No branches or pull requests

2 participants