-
Notifications
You must be signed in to change notification settings - Fork 4
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
TP2000-1083: Quota order number create #1081
Conversation
2076aa1
to
f78fc6a
Compare
quotas/forms.py
Outdated
def set_initial_data(self, *args, **kwargs): | ||
self.fields["category"].initial = self.instance.category |
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.
Would self.instance
always be None
in the case of this create form?
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.
Yeah not sure why this didn't blow up. It can be removed
path( | ||
f"quotas/create/", | ||
views.QuotaCreate.as_view(), | ||
name="quota-ui-create", | ||
), | ||
path( | ||
f"quotas/<sid>/confirm-create/", | ||
views.QuotaConfirmCreate.as_view(), | ||
name="quota-ui-confirm-create", | ||
), |
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 think get_ui_paths()
can automatically generate these routes so long as the corresponding views exist.
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've been avoiding using that because it obscures how the urls are defined. I'd much rather have them declared explicitly.
]) | ||
}} | ||
{{ workbasket_column("Regulations", [ | ||
{"text": "Create new regulation", "url": url('regulation-ui-create')}, |
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.
{"text": "Create new regulation", "url": url('regulation-ui-create')}, | |
{"text": "Create a new regulation", "url": url('regulation-ui-create')}, |
to match the other links?
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.
Oh I didn't notice this. Yeah I had just copy pasted and changed the model name. This probably reads better
@@ -105,6 +105,20 @@ def get_queryset(self): | |||
return models.QuotaOrderNumber.objects.approved_up_to_transaction(tx) | |||
|
|||
|
|||
class QuotaCreate(QuotaOrderNumberMixin, CreateTaricCreateView): |
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.
Would it be worth adding validation for business rules ON1 and ON2?
bf37bb5
to
02a60f8
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1081 +/- ##
==========================================
- Coverage 92.80% 92.79% -0.01%
==========================================
Files 460 460
Lines 34941 35083 +142
Branches 2674 2692 +18
==========================================
+ Hits 32426 32557 +131
- Misses 1992 1995 +3
- Partials 523 531 +8 ☔ View full report in Codecov by Sentry. |
TP2000-1083: Quota order number create
What