Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MNOE-616] Impac- Selection of currencies #334

Open
wants to merge 4 commits into
base: 2.0
Choose a base branch
from
Open

[MNOE-616] Impac- Selection of currencies #334

wants to merge 4 commits into from

Conversation

enizor
Copy link
Contributor

@enizor enizor commented Sep 21, 2017

In Account => Currencies, the user is able to choose which currencies will appear on the Impac dropdown, and the default currency for created dashboards.

It allows the user to have other settings (working on default_widget_currency and currencies)
The currencies list is taken from the config (set in admin panel/general/dashboard)
It sets the user.settings.currencies and .default_dashboard_currency attributes.
Currently the 'save' button does save the settings but does not load them in the frontend, a refresh is needed.
Copy link
Contributor

@alexnoox alexnoox left a comment

Choose a reason for hiding this comment

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

Please change the dashboard-currency-select directive to a component (https://docs.angularjs.org/guide/component)

@@ -31,7 +31,7 @@ angular.module 'mnoEnterpriseAngular'
bodyText: 'mno_enterprise.templates.components.language_selectbox.confirm'
headerTextExtraData: { name: locale.name }
bodyTextExtraData: { name: locale.name }
actionCb: -> MnoeCurrentUser.update(settings: {locale: $scope.selectedLangKey})
actionCb: -> MnoeCurrentUser.update(settings: Object.assign(MnoeCurrentUser.user.settings, locale: $scope.selectedLangKey)) # WIll destroy the whole previous settings.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous call would reset the whole settings settings attribute. Same logic is applied on the currency settings save action. I don't really understand how it could be improved, except by creating an API call for settings modification

angular.module 'mnoEnterpriseAngular'
.component('dashboardCurrencySelection', {
templateUrl: 'app/components/dashboard-currency-select/dashboard-currency-select.html',
controllerAs: "currencySelectCtrl"
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this configuration and use $ctrl in the template

isSelected = (currency) ->
currency.selected
# List of availables currencies
vm.currencies = DASHBOARD_CONFIG.impac.currencies.replace(/ /g,'').split(",").map( (c) -> { id: c, selected: true })
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove empty space before (c)

controller: (MnoeCurrentUser, DASHBOARD_CONFIG) ->
vm = this
isSelected = (currency) ->
currency.selected
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be on one line

# Is there at least one currency in common with the user' settings?
if vm.currencies.some((c,i,a) -> MnoeCurrentUser.user.settings.currencies.includes(c.id) )
# select the intersection of available currencies from the tenant and the settings
vm.currencies.map( (c) -> c.selected = MnoeCurrentUser.user.settings.currencies.includes(c.id))
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove empty space before (c)

@@ -809,7 +809,8 @@
"mno_enterprise.templates.dashboard.account.passwd_form_confirm": "Confirm Password",
"mno_enterprise.templates.dashboard.account.passwd_form_save": "Save",
"mno_enterprise.templates.dashboard.account.passwd_form_cancel": "Cancel",
"mno_enterprise.templates.dashboard.account.settings": "Settings",
"mno_enterprise.templates.dashboard.account.locale": "Locale",
"mno_enterprise.templates.dashboard.account.currencies": "Currencies ",
Copy link
Contributor

Choose a reason for hiding this comment

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

Empty space in locale

<label>{{ 'mno_enterprise.templates.components.currency_select.checkbox' | translate }}</label>
<div ng-repeat="currency in currencySelectCtrl.currencies">
<label>
<input type="checkbox" ng-model="currency.selected" style="">
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove empty style=""

@alexnoox alexnoox requested a review from ouranos October 19, 2017 04:55
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