Skip to content

[IMPAC-639] Add currency conversion to widget KPIs#400

Closed
xaun wants to merge 9 commits intomaestrano:release/v1.6.1from
xaun:feature/693-threshold-currency
Closed

[IMPAC-639] Add currency conversion to widget KPIs#400
xaun wants to merge 9 commits intomaestrano:release/v1.6.1from
xaun:feature/693-threshold-currency

Conversation

@xaun
Copy link
Copy Markdown
Contributor

@xaun xaun commented Sep 5, 2017

No description provided.

@xaun xaun requested a review from cesar-tonnoir September 5, 2017 21:45
@xaun
Copy link
Copy Markdown
Contributor Author

xaun commented Sep 5, 2017

@xaun xaun changed the title Feature/693 threshold currency [IMPAC-693] Add currency conversion to widget KPIs Sep 6, 2017
@cesar-tonnoir cesar-tonnoir changed the title [IMPAC-693] Add currency conversion to widget KPIs [IMPAC-639] Add currency conversion to widget KPIs Sep 8, 2017
@cesar-tonnoir cesar-tonnoir mentioned this pull request Sep 11, 2017
5 tasks
Copy link
Copy Markdown
Contributor

@cesar-tonnoir cesar-tonnoir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last two commits look ok, but we need to merge IMPAC-622 before this PR can be merged.


@update = (widget, opts, needContentReload = true) ->
widget.isLoading = needContentReload
widget.isLoading = needContentReload unless widget.isLoading
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'widget.isLoading ||= needContentReload' ?

kpi.watchables = [kpi.element_watched].concat(kpi.extra_watchables || [])

# Logic specific to applying newly fetched data to a dhb KPI.
# TODO: Redesign all business logic for v1 / v2 dhb & widget KPIs.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not really helpful: can you describe a bit more what should be reworked? What part of the current business logic is causing trouble?

@xaun xaun closed this Sep 22, 2017
@xaun xaun deleted the feature/693-threshold-currency branch September 29, 2017 13:15
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