-
Notifications
You must be signed in to change notification settings - Fork 103
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
Feat(invoice_custom_sections): use custom sections in pdf #3013
Feat(invoice_custom_sections): use custom sections in pdf #3013
Conversation
28e44be
to
30325ba
Compare
f1dfb75
to
7beee55
Compare
30325ba
to
4ea553a
Compare
6687849
to
93310f4
Compare
.joins('LEFT JOIN invoice_custom_section_selections ON invoice_custom_sections.id = invoice_custom_section_selections.invoice_custom_section_id | ||
AND invoice_custom_section_selections.customer_id is NULL') | ||
.order( | ||
Arel.sql('CASE WHEN invoice_custom_section_selections.id IS NOT NULL THEN 0 ELSE 1 END'), | ||
:name |
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.
No blocker here but maybe this should be turned into a scope. What do you think?
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 it's not needed, because this sorting - by selected for organization, and then alphabetically - only needed in the graphql response... I can't see where else it will be useful... or do you mean it will be better for readability?
(in all other places we're usually using the selected sections and only sort them alphabetically)
2c5755f
into
feat-invoice-custom-sections-create-applied-service
Use applied invoice custom sections in PDFs and include them in invoice payload
Also includes fixes found after QA