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-107] Fix admin finance page #75

Merged
merged 2 commits into from
Sep 8, 2016

Conversation

ouranos
Copy link
Contributor

@ouranos ouranos commented Sep 7, 2016

Tenant#current_billing_amount to be calculated via mnohub for performance reason (and also to
perform the currency conversion)

Admin Finance page:

  • Fix invoices table amount
  • Add extra finance KPIs

{current_billing_amount: {amount: billing.amount, currency: billing.currency_as_string}}
end
render json: data
tenant = MnoEnterprise::Tenant.get('tenant')
Copy link
Contributor

Choose a reason for hiding this comment

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

To get a singular resource, do we need to specify a fake id? Could it be MnoEnterprise::Tenant.get ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not a fake id actually. It's the resource name:

>> MnoEnterprise::Tenant.get('tenant')
=> GET /api/mnoe/v1/tenant

I'll check if I can find something cleaner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BrunoChauvet @x4d3
How about:

module MnoEnterprise
  class Tenant < BaseResource
    def self.show
      self.get('tenant')
    end
  end
end

So we can use MnoEnterprise::Tenant.show in the controller? Or I could override get?

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks cleaner indeed 👍

@BrunoChauvet
Copy link
Contributor

I would just add a TODO comment with the required version of mnohub so it's easier to do some clean up (don't forget to add the same todo to the old_tenant factory)

@ouranos
Copy link
Contributor Author

ouranos commented Sep 8, 2016

@BrunoChauvet see last commit

@BrunoChauvet
Copy link
Contributor

Looks good

Olivier Brisse added 2 commits September 8, 2016 16:45
This should be calculated via mnohub for performance reason (and also to
perform the currency conversion)
- Fix invoices table amount
- Add extra finance KPIs
@ouranos ouranos force-pushed the feature/107-fix-admin-finance branch from b8917c7 to 41e42a9 Compare September 8, 2016 06:45
@ouranos
Copy link
Contributor Author

ouranos commented Sep 8, 2016

Squashed

@ouranos ouranos merged commit db871cc into maestrano:3.0 Sep 8, 2016
@ouranos ouranos deleted the feature/107-fix-admin-finance branch September 8, 2016 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants