-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feature/46 product nl api #25
Conversation
a29bdbd
to
fe12f0a
Compare
@Coperh please review this PR today, this so we can prep an intermediate release of OP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Three things:
- Product admin list crashes if there is any product.
- Default admin index has onderwerp which no longer exists ( Probably renamed to thema)
- The end date can be set to before the start date. I assume you don't want this and I think you should prevent it from happening now instead of in the future
start_datum = factory.Faker("date") | ||
eind_datum = factory.Faker("date") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was looking at here and the end date can be before the start date. The model and admin also allow it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made start & end date optional on the model as that would've happened in a future pr anyway and added validation so that they cannot be the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I more meant that this is valid:
start_datum = datetime.date(2024, 1, 2)
eind_datum = datetime.date(2024, 12, 31)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think the start/end date is an issue but i'll approve
Fixes #
Changes