Adapt article & group sorted order PDF (solves #49) by twothreenine · Pull Request #87 · foodcoops/foodsoft_hackathon

twothreenine · 2024-07-29T17:23:54Z

Since there weren't any "ready for PR" issues left which I could work on (realistically), I worked on foodcoops/foodsoft#1099.

I also adapted the PDF sorted by groups to show the ordered amount in group order unit and the received amount in billing unit.
Matrix PDF remains to be adapted.

If you think this isn't ready to be merged, we can also keep it as a draft and later merge it on upstream (in order to focus on the PR).

…on chain instead of a conversion table

- made article model spec work again - removed some more dead shared db code

Explanation of why this only occurs now: The previous rails version ignored invalid ids for `accepts_nested_attributes_for` parameters, the new one doesn't. -> We need to avoid sending them.

This also fixes several actual bugs (partially caused by the rabase) detected by the specs

Changed decimals to 3 as requested

…onvertability (#70) * On #42: Fax pdf/csv: Decimals dependant on supplier_order_unit's si convertability * Solve #42: Improve fax PDF, CSV, text - outsource format_units_to_order to OrderHelper - fax text: include unit, adjust column width - fax PDF & text: only include order number if any present * On #42: - Adapted order_txt to generalized creating the text table and added spec - Code style fixes for order_fax * On #42 Fixes error dad0bb9#r143648091 --------- Co-authored-by: twothreenine <leonard_ostler@yahoo.de>

Closes #16 Merged the hackathon and small seeds and updated the articles to handle the new unit system. Dropped the .nl and .de seed files, because I think it's unnecessary work to further maintain them.

* Fixes #66 * Fixes broken integration tests * chore: refactor extract articles_helper article_version test setup function * chore: make helper functions private * On #66 Fixes https://github.com/foodcoopsat/foodsoft_hackathon/pull/74/files/b078cfaa87d334dba8eaafff5b2be152f293c448#r1670426999 --------- Co-authored-by: Philipp Rothmann <philipprothmann@posteo.de>

- rearrange CSV columns as I suggested in #47 - add locales for column headings according to my suggestions in #50 (to do: adjust terms across menus -- post-merge?) - update documentation of CSV layout (#46) TO DO: any adaptations for rearranged tables necessary which I've overlooked? (for example sync feature)

coveralls · 2024-07-29T17:31:20Z

Pull Request Test Coverage Report for Build 10178232479

Details

10 of 36 (27.78%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.2%) to 67.893%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
app/documents/order_by_articles.rb	0	9	0.0%
app/lib/order_pdf.rb	6	23	26.09%

Totals
Change from base Build 10113367318:	-0.2%
Covered Lines:	4688
Relevant Lines:	6905

💛 - Coveralls

twothreenine · 2024-08-02T09:35:08Z

I don't think this would make the fork "bigger" as it only affects files already affected by the fork.

lentschi · 2024-08-03T14:58:17Z

I don't think this would make the fork "bigger" as it only affects files already affected by the fork.

Yeah, but it affects more considerably more lines in those files. Also none of this is unit tested and thus neither is you new "total" logic (which btw. has nothing to do with the units fork).

As for the change itself: It looks good to me in general. I'm just not sure, if everyone would agree with the visible changes in the pdf.

Cons I see (admittedly minor):

More text to read - a lot of it repetitive (E.g. if many groups order beer, you will have just as many occurrences of <amount> Bottle (0.5l), which is a bit annoying)
Ordered and Received columns are no longer easily comparable. (E.g. for the hackathon "Smoked tofu": 2 Piece (160 g) |320.000 g. For many numbers, this might be arduous.)

Long story short: I'm not saying that in total this PR wouldn't be an improvement, I just think it should rather be done and discussed upstream.

lentschi and others added 30 commits July 26, 2024 10:34

Article units squash

ecf7c17

Fix typos and removed dead code

cfbe5b7

Remove obsolete testing shared db along with its tasks

2a42360

Added no-upstream for byebug

539613b

On #47 Adapt unit ratio export/import to be interpreted as a conversi…

ba11e4b

…on chain instead of a conversion table

On #21: stock orders: prohibit 'ordering' more than what's on stock

5bcea4a

Started with #8

b39cb77

- made article model spec work again - removed some more dead shared db code

Fixes #53

51c2703

Explanation of why this only occurs now: The previous rails version ignored invalid ids for `accepts_nested_attributes_for` parameters, the new one doesn't. -> We need to avoid sending them.

fix: round article quantity in order article pdf

37d1b27

Don't show articles migrate button for newly created suppliers

d831092

On #8: All existing specs pass now

acc982d

This also fixes several actual bugs (partially caused by the rabase) detected by the specs

Rubocop fixes

e5a2398

First proposal for #35

74ac2f3

On #15: Open with inset button instead of on every focus

cb38646

Fixes #56

8b6d01a

On #35: Adapted list of default units

90c4fcb

Fixes #42

1c9612c

Fixes #42 again

4106ee8

Changed decimals to 3 as requested

On #8: Implemented article version model unit tests

0abb392

On #8: Adapted article version unit tests

eb33fe8

On #35 Added missing metric default unit: GRM

8ed4bb9

Fixes #56 again

be44773

Fixes #59

6eb79bf

Fixes #58

a51304b

Fixes #60

3939556

Fixes #65

3941539

Fixes #62

6c77080

On #8: Fixes failing tests on github

476e322

On #8: Article units integration tests

3ce6390

On #8: Started with unit tests for supplier article sharing

c67a465

lentschi and others added 21 commits July 26, 2024 10:34

Fixes #68

de4792a

Fixes #67

b61b847

Fixes #69

2e19cd3

On #8: Done with sync specs

2c02208

On #15: Fixes conversion popover min-width

67041a1

Fixes #61

2ae2c6d

Fixes #73

0754f15

Better alias for #71

222ada6

On #64: Add missing unit labels

97a3f48

feat: update seed files (#76)

366065f

Closes #16 Merged the hackathon and small seeds and updated the articles to handle the new unit system. Dropped the .nl and .de seed files, because I think it's unnecessary work to further maintain them.

Workaround for #78

61a2b1a

Moved fork's migrations after rebase on upstream/master

681095b

Fixes #85

dec2762

Allow price calculation for new records

9191f42

Fixes #83

3871c37

Another workaround for #78

756fda0

Fixes #81

b3bc712

Fix unit tests (broken since 3871c37)

3297e5e

twothreenine requested a review from lentschi July 29, 2024 17:23

Adapt article & group sorted order PDF (solves #49)

a8d87d7

twothreenine force-pushed the feature/adapt-article+group-order-pdf branch from 9594c1c to a8d87d7 Compare July 31, 2024 10:07

lentschi force-pushed the master branch from 1755d30 to f4fe06c Compare October 11, 2024 10:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt article & group sorted order PDF (solves #49)#87

Adapt article & group sorted order PDF (solves #49)#87
twothreenine wants to merge 52 commits intomasterfrom
feature/adapt-article+group-order-pdf

twothreenine commented Jul 29, 2024

Uh oh!

coveralls commented Jul 29, 2024 •

edited

Loading

Uh oh!

twothreenine commented Aug 2, 2024

Uh oh!

lentschi commented Aug 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

twothreenine commented Jul 29, 2024

Uh oh!

coveralls commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 10178232479

Details

💛 - Coveralls

Uh oh!

twothreenine commented Aug 2, 2024

Uh oh!

lentschi commented Aug 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Jul 29, 2024 •

edited

Loading