Enhanced matrix pdf #489

FcFlodo · 2017-08-17T16:17:40Z

sort articles by description for easier localization
rotate header for enhanced readability and longer descriptions
add net price for easier check with supplier invoice
add unit to header for quantity-clarification

A bug in the prawn gem forced me to update the gem
prawnpdf/prawn#409
prawnpdf/prawn-table#32

I have uploaded the Gemfile.lock which has changed, but I am not sure if this has to be changed?

…it to header

paroga

Many thanks for your contribution! I'll check the changes on my local machine later, but I've added already some comments, if you want to address them in the meantime.

paroga · 2017-08-17T16:23:20Z

config/locales/en.yml

@@ -734,6 +734,7 @@ en:
      - Article
      - Unit
      - Unit quantity
+      - Netto price


if you compare it to the other translations "Nettopreis" in german is usually "Price" in english

please also update the other languages. the translation should be easy to find

paroga · 2017-08-17T16:24:59Z

Gemfile.lock

@@ -217,7 +224,7 @@ GEM
    libv8 (3.16.14.17)
    loofah (2.0.3)
      nokogiri (>= 1.5.9)
-    mail (2.6.6)
+    mail (2.6.4)


are these version changes to a lower version really necessary? i don't think so

No they are not necessary. The Gemfile.lock changed automatically and I was not sure if I should upload it.

If you would like to remove these changes, that would clean it up a bit (and avoid a conflict).
Or do you need a newer prawn? If you do, please rebase on current master.

paroga · 2017-08-17T16:28:14Z

app/documents/order_matrix.rb


    text I18n.t('documents.order_matrix.heading'), style: :bold
    move_down 5
    text I18n.t('documents.order_matrix.total', :count => order_articles.size), size: fontsize(8)
    move_down 10

    order_articles_data = [I18n.t('documents.order_matrix.rows')]
+


please avoid unnecessary whitespace changes

paroga · 2017-08-17T16:28:37Z

app/documents/order_matrix.rb

                              number_with_precision(article_price(a), precision: 2),
                              a.units]
    end
+
+    #order_articles_data.sort_by!{|a| a[0].downcase}


please avoid useless comments

paroga · 2017-08-17T16:30:53Z

app/documents/order_matrix.rb

-        name = name.split.collect { |w| w.truncate(8) }.join(" ")
-        header << name.truncate(30)
+        name = name.split.collect { |w| w.truncate(20) }.join(" ")
+        header << name.truncate(35)+' - ('+header_article.article.unit+')'


header << "#{name.truncate(35)} - (#{header_article.article.unit})" might be easier to read

paroga · 2017-08-17T16:36:11Z

app/documents/order_matrix.rb

@@ -13,22 +13,26 @@ def title
  end

  def body
-    order_articles = @order.order_articles.ordered
+    order_articles = @order.order_articles.sort_by{|a| a.article.name.downcase}


can we do this sorting via ActiveRecord instead?

I did the sorting still with sort_by, because I was afraid, that I make unwanted changes in the database with ActiveRecord sorting. Feel free to change this if this is a problem.

Sorting belongs in the database, this is a memory hog and doesn't scale for large orders. Please reconsider the ordered scope if you think a different sort order is useful everywhere; if it only makes sense here, don't use the .ordered scope but use active-record sorting. If you'd like case-insensitive sorting, indexes may also be needed.

Sorry for the condensed answer, if you need more explanation, please don't hesitate to ask. We'll do our best to help!

paroga · 2017-08-17T16:39:18Z

app/documents/order_matrix.rb

+
+      table.cells.border_width = 1
+      table.cells.border_color = '666666'
+      table.row_colors = ['ffffff','ececec']


you removed the indentation for this block

paroga · 2017-08-17T16:52:53Z

Gemfile

@@ -22,7 +22,7 @@ gem 'rails-i18n'

 gem 'mysql2'
 gem 'prawn'
-gem 'prawn-table'
+gem 'prawn-table', :git => 'https://github.com/straydogstudio/prawn-table.git', ref: '759a27b6'


this revision is from 2014. the latest upstream version is from 2016. do you see any chance that the commit from straydogstudio gets accepted in upstream? I do not like the idea of using such an outdated version. If the merge in upstream is not possible: Could you rebase the commit from straydogstudio to upstream master, publish them in a new repository and use that in the Gemfile?

wvengen · 2017-09-30T15:55:04Z

Hi @FcFlodo, thanks for your contribution! Do you see any opportunity to finish it?

FcFlodo · 2017-10-08T11:14:24Z

Hi @wvengen and @paroga !
Sorry for the long delay, I couldn´t spare much time in the past few weeks for this project.

I tried to get my code running with the new prawn-version, but until now I haven´t found a way to solve this.

wvengen · 2017-10-09T21:02:17Z

Hi @FcFlodo, no problem. What about removing the header rotation for now, so that the other improvements can be included? If then an solution is found for the issue, the rotated headers (which I can imagine would be much much nicer) can be included.

I also suggest to revive the discussion upstream. If there are others interested in the feature, it may get some traction. If not, then you could try to fix it upstream and get it included. Putting it into a separate repository would not have my preference, because that basically means that we'd need to maintain it.

…oved added whitespaces etc.

FcFlodo · 2017-10-10T16:36:34Z

Finished my contribution without the rotated headers. I will ask if the changes made by straydogstudio in the prawn-table gem could be implemented.

wvengen

Thanks for the update! I still have some remarks.
To get an idea of how the changes would look, would you be able to show the differences visually, e.g. as screenshot(s) or an upload of the new pdf? That would help reviewing. Thanks!

wvengen · 2017-10-27T15:27:57Z

Gemfile.lock

@@ -217,7 +224,7 @@ GEM
    libv8 (3.16.14.17)
    loofah (2.0.3)
      nokogiri (>= 1.5.9)
-    mail (2.6.6)
+    mail (2.6.4)


If you would like to remove these changes, that would clean it up a bit (and avoid a conflict).
Or do you need a newer prawn? If you do, please rebase on current master.

wvengen · 2017-10-27T15:30:36Z

app/documents/order_matrix.rb

@@ -13,22 +13,26 @@ def title
  end

  def body
-    order_articles = @order.order_articles.ordered
+    order_articles = @order.order_articles.sort_by{|a| a.article.name.downcase}


Sorting belongs in the database, this is a memory hog and doesn't scale for large orders. Please reconsider the ordered scope if you think a different sort order is useful everywhere; if it only makes sense here, don't use the .ordered scope but use active-record sorting. If you'd like case-insensitive sorting, indexes may also be needed.

Sorry for the condensed answer, if you need more explanation, please don't hesitate to ask. We'll do our best to help!

paroga · 2017-11-15T20:24:01Z

@FcFlodo: I did a bigger rewrite of the OrderPdf classes in #532. Does it solve your issues too?

carermey · 2019-08-28T08:07:58Z

hello.

we are using the foodsoft for our big wholesaler ordering! this extension:

sort articles by description for easier localization
rotate header for enhanced readability and longer descriptions
add net price for easier check with supplier invoice
add unit to header for quantity-clarification

would be absolute great !

If the articles would be ordert first by categorie and afterwards by produktname in alphabetical order would make the overview also much better !

Thank you a lot for your work on it !

Carermey

yksflip · 2023-07-03T13:49:12Z

Hey,
this look's stale to me, is this still relevant?
If so, please feel free to reopen this!
Thanks again for your contribution!

matrix pdf: sort by description, rotate header, add net price, add un…

6a40798

…it to header

paroga requested changes Aug 17, 2017

View reviewed changes

added languages in locales, removed header rotation (bug in gem), rem…

52a51e5

…oved added whitespaces etc.

wvengen requested changes Oct 27, 2017

View reviewed changes

wvengen added the needs work label Oct 27, 2017

paroga force-pushed the master branch from 0ad596c to 3b18dc7 Compare November 6, 2017 19:14

paroga force-pushed the master branch 6 times, most recently from ca76347 to c6250de Compare September 5, 2020 15:00

paroga force-pushed the master branch from ea9d5cf to cdc9e91 Compare February 18, 2022 11:47

yksflip closed this Jul 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced matrix pdf #489

Enhanced matrix pdf #489

FcFlodo commented Aug 17, 2017

paroga left a comment

paroga Aug 17, 2017

paroga Aug 17, 2017

FcFlodo Aug 19, 2017

wvengen Oct 27, 2017

paroga Aug 17, 2017

paroga Aug 17, 2017

paroga Aug 17, 2017

paroga Aug 17, 2017

FcFlodo Oct 10, 2017

wvengen Oct 27, 2017

paroga Aug 17, 2017

paroga Aug 17, 2017

wvengen commented Sep 30, 2017

FcFlodo commented Oct 8, 2017

wvengen commented Oct 9, 2017

FcFlodo commented Oct 10, 2017

wvengen left a comment

wvengen Oct 27, 2017

wvengen Oct 27, 2017

paroga commented Nov 15, 2017

carermey commented Aug 28, 2019

yksflip commented Jul 3, 2023

Enhanced matrix pdf #489

Enhanced matrix pdf #489

Conversation

FcFlodo commented Aug 17, 2017

paroga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wvengen commented Sep 30, 2017

FcFlodo commented Oct 8, 2017

wvengen commented Oct 9, 2017

FcFlodo commented Oct 10, 2017

wvengen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paroga commented Nov 15, 2017

carermey commented Aug 28, 2019

yksflip commented Jul 3, 2023