Journal export csv with custom options #1554

mbayopanda · 2017-04-24T15:13:55Z

This PR allows to:

Export to csv the trial balance
Customize the name of the csv file
Use the exportation tool with grids

ghost · 2017-04-27T06:54:06Z

☔ The latest upstream changes (presumably 033d9cd) made this pull request unmergeable. Please resolve the merge conflicts.

ghost · 2017-04-28T11:49:48Z

☔ The latest upstream changes (presumably ac06db3) made this pull request unmergeable. Please resolve the merge conflicts.

ghost · 2017-05-02T18:04:52Z

☔ The latest upstream changes (presumably 9f17d54) made this pull request unmergeable. Please resolve the merge conflicts.

mbayopanda · 2017-05-04T11:37:46Z

@jniles could you review this PR

jniles · 2017-05-04T11:39:29Z

Working on it!

ghost · 2017-05-08T08:42:46Z

☔ The latest upstream changes (presumably 4df53c2) made this pull request unmergeable. Please resolve the merge conflicts.

ghost · 2017-05-08T11:39:55Z

☔ The latest upstream changes (presumably 7cf8682) made this pull request unmergeable. Please resolve the merge conflicts.

ghost · 2017-05-09T09:26:11Z

☔ The latest upstream changes (presumably 6bc89ba) made this pull request unmergeable. Please resolve the merge conflicts.

…into fix_journal_export

mbayopanda · 2017-05-10T09:21:52Z

@jniles could you review this PR please

jniles · 2017-05-04T11:38:02Z

client/src/i18n/fr/posting_journal.json

@@ -60,7 +60,7 @@
 "MISSING_DATES":"Date(s) manquante(s)",
 "MISSING_DOCUMENT_ID":"Ligne(s) sans ID Document",
 "MISSING_ENTITY":"Ligne(s) sans ID entite",
-"MISSING_FISCAL_OR_PERIOD":"Annee fiscale ou periode omise",
+"MISSING_FISCAL_OR_PERIOD":"Année fiscale ou periode omise",


jniles · 2017-05-04T11:41:01Z

client/src/js/app.js

@@ -5,7 +5,8 @@ var bhima = angular.module('bhima', [
  'tmh.dynamicLocale', 'ngFileUpload', 'ui.grid',
  'ui.grid.selection', 'ui.grid.autoResize', 'ui.grid.resizeColumns',
  'ui.grid.edit', 'ui.grid.grouping', 'ui.grid.treeView', 'ui.grid.cellNav',
-  'ui.grid.pagination', 'ui.grid.moveColumns', 'angularMoment', 'ngMessages',
+  'ui.grid.pagination', 'ui.grid.moveColumns', 'ui.grid.exporter',


Hmm... is this possible to do without the exporter service? I don't see where it is used and I hate to introduce another dependency for something we can almost already do...

Yes but with a lot of supplementary request to the server, also if we don't use the exporter service we cannot share the client service for exporting with custom name to other modules.

The first reason of using exporter service was because the trial balance request is so complex, and using this request again and again is not useful when we don't have a lot of data at client and data are already at the client we have just to export them.

Also exporter service give us more options for exportation which are flexible and we can use them in other modules with less effort

Your analysis is correct - for the trial balance, it is more work to export a CSV from the server side.

My biggest worry with this change is that, as a developer, I do not know when to use the server-side CSV rather than the client-side CSV. It creates another question to ask during development and something else to test....

jniles · 2017-05-04T17:37:32Z

client/src/js/services/grid/GridExport.js

+  function GridExport(gridOptions, defaultRows, defaultCols) {
+
+    this.gridOptions = gridOptions;
+    this.ROWS = defaultRows;


It took me a little while to realize that these aren't actually rows .. it's a string. Could you name it something like defaultRowKey or something?

jniles · 2017-05-04T17:37:59Z

client/src/modules/journal/journal.js

@@ -50,6 +50,9 @@ function JournalController(Journal, Sorting, Grouping,
  // top level cache
  var cache = AppCache(cacheKey + '-module');
  var vm = this;
+
+  var gridApi;


I don't think you use gridApi here.

jniles · 2017-05-04T17:38:44Z

client/src/modules/journal/modals/trialBalanceError.body.js

+    headerCellFilter : 'translate',
+    headerCellClass  : cssClass,
+  }, {
+    field            : 'trans_id',


👍 Good catch.

jniles · 2017-05-04T17:48:05Z

client/src/modules/templates/modals/export.modal.js

+  var gridOptions = Data.options || {};
+  var gridApi = Data.api || {};
+  var filename = Data.filename || 'Export ' + moment().format('YYYY-MM-DD');
+  var ROWS = Data.rows || 'visible';


For simplicity, I propose we always render all rows in the grid. That's how our PDFs work and it makes sense that the filters chosen will be the ones applied to the dataset.

The current state of the exporter is just for csv, so we render what is visible to the client with the possibility of choosing other options (selected or all). This flexibility is useful because we have some grid which has grouped data and other not, for grid which has grouped data with the option visible the service export what is visible (ex. only grouping rows in the case of posting journal with grouped rows) and the user can select other options if he want, and developers can set the default option for grid according needs.

…into fix_journal_export

mbayopanda · 2017-05-11T09:04:54Z

@jniles could you review this PR

jniles

@mbayopanda, I need a second opinion on this.

I'm concerned about having two ways of doing things - one on the client and one on the server. It feels like unnecessary code bloat and technical debt... kind of against both DRY and KISS principles.

@sfount, could you provide your perspective?

jniles · 2017-05-11T09:16:04Z

client/src/js/app.js

@@ -5,7 +5,8 @@ var bhima = angular.module('bhima', [
  'tmh.dynamicLocale', 'ngFileUpload', 'ui.grid',
  'ui.grid.selection', 'ui.grid.autoResize', 'ui.grid.resizeColumns',
  'ui.grid.edit', 'ui.grid.grouping', 'ui.grid.treeView', 'ui.grid.cellNav',
-  'ui.grid.pagination', 'ui.grid.moveColumns', 'angularMoment', 'ngMessages',
+  'ui.grid.pagination', 'ui.grid.moveColumns', 'ui.grid.exporter',


Your analysis is correct - for the trial balance, it is more work to export a CSV from the server side.

My biggest worry with this change is that, as a developer, I do not know when to use the server-side CSV rather than the client-side CSV. It creates another question to ask during development and something else to test....

ghost · 2017-05-11T10:33:24Z

☔ The latest upstream changes (presumably 89a3272) made this pull request unmergeable. Please resolve the merge conflicts.

jniles · 2017-05-15T10:34:18Z

Since the work has already been done for this, we'll merge it and see how it works in production. If necessary, we can revert changes or implement something different/more.

…into fix_journal_export

mbayopanda requested a review from jniles April 24, 2017 15:24

Journal export csv with custom options

9a0e98b

mbayopanda requested review from sfount and removed request for jniles April 26, 2017 09:03

mbayopanda added 4 commits April 27, 2017 13:04

force update the branch

b50308f

Fix mergin conflicts

e287caf

Merge branch 'master' into fix_journal_export

a2b948c

Merge branch 'master' into fix_journal_export

aedf2c1

mbayopanda added 5 commits May 2, 2017 12:07

Fix mergin conflicts

fddcb14

Fix translation missing

96686b2

Merge branch 'master' into fix_journal_export

3ec8486

Merge branch 'master' into fix_journal_export

60704b2

Fix mergin conflicts

49003e8

mbayopanda added 8 commits May 4, 2017 10:02

Merge branch 'master' into fix_journal_export

5b3830a

Merge branch 'master' into fix_journal_export

4d663fd

Merge branch 'master' into fix_journal_export

a4fb28c

Journal export csv with custom options

9c4be51

force update the branch

5a0eba3

Fix translation missing

8081ae8

Fix mergin conflict

54341ac

Merge branch 'master' into fix_journal_export

9b18d21

mbayopanda added 2 commits May 8, 2017 10:43

Fix mergin conflicts

63c1383

fix mergin conflicts

a025765

Merge branch 'master' into fix_journal_export

e21c788

mbayopanda added 5 commits May 9, 2017 11:09

Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima-2.X …

07d7e3b

…into fix_journal_export

fix mergin conflicts

31f4662

Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima-2.X …

0301dcc

…into fix_journal_export

Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima-2.X …

d2bec39

…into fix_journal_export

Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima-2.X …

06a81eb

…into fix_journal_export

jniles reviewed May 10, 2017

View reviewed changes

mbayopanda added 2 commits May 10, 2017 13:24

Fix and enhancements

3d79a76

Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima-2.X …

45cb46f

…into fix_journal_export

jniles reviewed May 11, 2017

View reviewed changes

Update and fix mergin conflicts

6d47968

jniles added the requires bower install label May 15, 2017

jniles merged commit 869599d into IMA-WorldHealth:master May 15, 2017

Merge branch 'master' of https://github.com/IMA-WorldHealth/bhima-2.X …

b3f050e

…into fix_journal_export

mbayopanda deleted the fix_journal_export branch August 22, 2017 14:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Journal export csv with custom options #1554

Journal export csv with custom options #1554

mbayopanda commented Apr 24, 2017 •

edited by jniles

Loading

ghost commented Apr 27, 2017

ghost commented Apr 28, 2017

ghost commented May 2, 2017

mbayopanda commented May 4, 2017

jniles commented May 4, 2017

ghost commented May 8, 2017

ghost commented May 8, 2017

ghost commented May 9, 2017

mbayopanda commented May 10, 2017

jniles May 4, 2017

jniles May 4, 2017

mbayopanda May 10, 2017

jniles May 11, 2017

jniles May 4, 2017

mbayopanda May 10, 2017

jniles May 4, 2017

jniles May 4, 2017

jniles May 4, 2017

mbayopanda May 10, 2017

mbayopanda commented May 11, 2017

jniles left a comment

jniles May 11, 2017

ghost commented May 11, 2017

jniles commented May 15, 2017

Journal export csv with custom options #1554

Journal export csv with custom options #1554

Conversation

mbayopanda commented Apr 24, 2017 • edited by jniles Loading

ghost commented Apr 27, 2017

ghost commented Apr 28, 2017

ghost commented May 2, 2017

mbayopanda commented May 4, 2017

jniles commented May 4, 2017

ghost commented May 8, 2017

ghost commented May 8, 2017

ghost commented May 9, 2017

mbayopanda commented May 10, 2017

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

mbayopanda commented May 11, 2017

jniles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented May 11, 2017

jniles commented May 15, 2017

mbayopanda commented Apr 24, 2017 •

edited by jniles

Loading