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

Show only suppliers with articles in the dropdown-menu for new orders #347

Merged
merged 1 commit into from Feb 18, 2015

Conversation

Projects
None yet
2 participants
@paroga
Member

paroga commented Feb 18, 2015

When there are many suppliers to be able to select them for invoices,
the menu for creating new orders gets unclear.

@wvengen

This comment has been minimized.

Show comment
Hide comment
@wvengen

wvengen Feb 18, 2015

Member

Nice idea! Does this generate N+1 queries for the article count? In that case it'd be useful to preload the article count. Or, better, update the controller to only return suppliers that have articles. Could be done using group_by and having.

Member

wvengen commented Feb 18, 2015

Nice idea! Does this generate N+1 queries for the article count? In that case it'd be useful to preload the article count. Or, better, update the controller to only return suppliers that have articles. Could be done using group_by and having.

@paroga

This comment has been minimized.

Show comment
Hide comment
@paroga

paroga Feb 18, 2015

Member

Does group_by and having help, when I need to "join" from an additional table?
In SQL I'd do sth like SELECT * FROM suppliers WHERE id in (SELECT supplier_id FROM articles WHERE undeleted)

Member

paroga commented Feb 18, 2015

Does group_by and having help, when I need to "join" from an additional table?
In SQL I'd do sth like SELECT * FROM suppliers WHERE id in (SELECT supplier_id FROM articles WHERE undeleted)

@wvengen

This comment has been minimized.

Show comment
Hide comment
@wvengen

wvengen Feb 18, 2015

Member

Plain SQL would also be an option:

Supplier.where(id: Article.undeleted.select(:supplier_id).distinct)

Using group by and having:

Supplier.joins(:articles).group(:supplier_id).having('COUNT(articles.id) > 0').last

Both materialize to one query. The first was faster for me.

Member

wvengen commented Feb 18, 2015

Plain SQL would also be an option:

Supplier.where(id: Article.undeleted.select(:supplier_id).distinct)

Using group by and having:

Supplier.joins(:articles).group(:supplier_id).having('COUNT(articles.id) > 0').last

Both materialize to one query. The first was faster for me.

Show only suppliers with articles in the dropdown-menu for new orders
When there are many suppliers to be able to select them for invoices,
the menu for creating new orders gets unclear.
@@ -6,7 +6,7 @@
= t '.new_order'
%span.caret
%ul.dropdown-menu
- Supplier.undeleted.order('suppliers.name ASC').each do |supplier|
- Supplier.where(id: Article.undeleted.select(:supplier_id).distinct).order('suppliers.name ASC').each do |supplier|

This comment has been minimized.

@wvengen

wvengen Feb 18, 2015

Member

Putting these queries in the view is bad practice. I'll move it to the controller when merged.

@wvengen

wvengen Feb 18, 2015

Member

Putting these queries in the view is bad practice. I'll move it to the controller when merged.

wvengen added a commit that referenced this pull request Feb 18, 2015

Merge pull request #347 from foodcoop1040/suppliers_with_articles
Show only suppliers with articles in the dropdown-menu for new orders

@wvengen wvengen merged commit 27fe8a4 into foodcoops:master Feb 18, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@paroga paroga deleted the paroga:suppliers_with_articles branch Feb 18, 2015

@wvengen wvengen modified the milestone: 4.4 Feb 27, 2015

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