Skip to content

[IMPAC-745] Fixed Cash Projection Crash #463

Closed
iseessel wants to merge 71 commits intomaestrano:1.6from
iseessel:bug/745-cash-projection-crash
Closed

[IMPAC-745] Fixed Cash Projection Crash #463
iseessel wants to merge 71 commits intomaestrano:1.6from
iseessel:bug/745-cash-projection-crash

Conversation

@iseessel
Copy link
Copy Markdown
Contributor

@cesar-tonnoir

Instead of calling update on highCharts multiple times new data is introduced, I created an entirely new chart. For some reason I could not figure out the create method is faster than the update for the highCharts API.

Because of this, I had to change the way the options were stored, as we need to put all the options in at once.

Additionally if an input was invalid, the create and cancel buttons would not go back to normal. I solved that by adding a tostr error and invoking #cancelCreateKpi. I don't know what the intended behavior was, but you could not choose a number less than 0.

I am also not entirely confident in how the options from the specific widgets is getting integrated into the highCharts factory. It seems like _.get is being used to retrieve options from the specific widgets and injecting them into the big options-hash in highcharts-factory.svc.coffee. I think this could be refactored to merely merging the specific widgets' options hash with these default options, but this would take some time. For this pull request, I followed the current patterns of merging options.

@iseessel iseessel changed the base branch from master to 1.6 January 26, 2018 17:43
marcocode and others added 27 commits January 30, 2018 21:28
- Statement and Balance in Xero displayed
- Personalised DemoData feature
Add max-height to allow overflow scroll to work.
Add ASCII encode/decoding for tag values
Widgets Templates whitelist / blacklist feature
[IMPAC-759] Fix cash_balance accounts list
[IMPAC-750] Fix widgets templates rendering order
[IMPAC-763] Fixed Accounts classes comparison Dates
[IMPAC-681] Fix tags to allow special chars
[IMPAC-757] Added radio mode for organization
@iseessel
Copy link
Copy Markdown
Contributor Author

@xaun

Along with your approach, I suggest we use short setter methods that take direct and specific arguments that populate the @options hash. Anything that is generated dynamically in @template will be refactored to a setter method that populates the @options hash.

Fix dependency issue, travis chrome launching, & dashboard.svc specs
@iseessel iseessel force-pushed the bug/745-cash-projection-crash branch from 588a348 to bc12307 Compare February 17, 2018 00:35
@iseessel
Copy link
Copy Markdown
Contributor Author

@ouranos Realized I was making PR onto 1.6 and not 1.6.9. This New Pull request made.

@iseessel iseessel force-pushed the bug/745-cash-projection-crash branch from 6c9bf95 to 415acc2 Compare February 20, 2018 20:05
@xaun
Copy link
Copy Markdown
Contributor

xaun commented Feb 21, 2018

Moved to PR #480

@xaun xaun closed this Feb 21, 2018
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.

3 participants