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

do not store GroupOrderArticles with zero quantity and tolerance #273

Merged
merged 1 commit into from
Mar 27, 2014

Conversation

wvengen
Copy link
Member

@wvengen wvengen commented Mar 18, 2014

In GroupOrderArticle there is a scope ordered which means both "those articles that have been ordered", as well as "ordered by group name". In other models, ordered is used as the latter.

Cleaning this up, I could not find a reason why to keep GroupOrderArticles where all quantities are zero. These records are currently the bulk of the records in this table, and consume quite some computation and disk access time (reference to #49).

I've tested this on a copy of a production database with 1117655 GroupOrderArticles, which were reduced to a mere 12372 after the migration (the migration took 5 seconds on my development machine).

@bennibu as this affects the ordering algorithm, would you please comment if this is a good thing to do?

p.s. in the show group order page, articles not ordered are still presented (unless hidden by javascript)

@bennibu
Copy link
Contributor

bennibu commented Mar 25, 2014

@wvengen, I can't remember exactly why we kept the "null results" of
GroupOrderArticles. I guess, this was for programming convenience, because
otherwise we had no result objects when fetching the GroupOrder result
in the GroupOrder#edit view.

I have not checked it yet, but I think it is a very good idea to clean
up the data.

Am 18.03.2014 17:55, schrieb wvengen:

In |GroupOrderArticle| there is a scope |ordered| which means both
"those articles that have been ordered", as well as "ordered by group
name". In other models, |ordered| is used as the latter.

Cleaning this up, I could not find a reason why to keep
|GroupOrderArticle|s where all quantities are zero. These records are
currently the bulk of the records in this table, and consume quite some
computation and disk access time (reference to #49
#49).

I've tested this on a production database with 1117655
|GroupOrderArticles|, which were reduced to a mere 12372 after the
migration (the migration took 5 seconds on my development machine).

@bennibu https://github.com/bennibu as this affects the ordering
algorithm, would you please comment if this is a good thing to do?


    You can merge this Pull Request by running

git pull https://github.com/wvengen/foodsoft feature-destroy_empty_goas

Or view, comment on, or merge it at:

#273

    Commit Summary


Reply to this email directly or view it on GitHub
#273.

pgp Schlüssel-ID: 0x3B2EE0A4
Fingerabdruck: 805F 73B1 9F45 4122 2FE6 ED75 0786 8427 3B2E E0A4

@wvengen
Copy link
Member Author

wvengen commented Mar 27, 2014

Thanks for your reply! I feel confident to continue with this change now.

wvengen added a commit that referenced this pull request Mar 27, 2014
do not store GroupOrderArticles with zero quantity and tolerance
@wvengen wvengen merged commit 7bf7e91 into foodcoops:master Mar 27, 2014
@wvengen wvengen deleted the feature-destroy_empty_goas branch March 27, 2014 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants