Support new endpoints and move Customer, Plan and Data Sources outside of Import namespace #32
Conversation
promulo
changed the title
| ds = ChartMogul::DataSource.create!(name: 'TestDS') | ||
| ds.send(:set_uuid, 'ds_5ee8bf93-b0b4-4722-8a17-6b624a3af072') | ||
|
|
||
| data_source = described_class.retrieve(ds.uuid) |
There was a problem hiding this comment.
Perhaps in this line you also need to use explicit uuid instead of method. Because who knows what happened when you did ds.send(:set_uuid,...):
data_source = described_class.retrieve('ds_5ee8bf93-b0b4-4722-8a17-6b624a3af072')
In general I think it is better to use explicit values in tests, so that if you change something, then you'll need to change test as well.
| ) | ||
| plan.send(:set_uuid, 'pl_5ee8bf93-b0b4-4722-8a17-6b624a3af072') | ||
|
|
||
| plan = described_class.retrieve(plan.uuid) |
There was a problem hiding this comment.
plan = described_class.retrieve('pl_5ee8bf93-b0b4-4722-8a17-6b624a3af072')
| plan.interval_count = 2 | ||
| plan.update! | ||
|
|
||
| plan = described_class.retrieve(plan.uuid) |
There was a problem hiding this comment.
same with uuid (that's my opinion, of course we can discuss if someone diasgrees)
|
|
||
| it 'retrieves existing data source matching uuid', uses_api: true do | ||
| ds = ChartMogul::DataSource.create!(name: 'TestDS') | ||
| ds.send(:set_uuid, 'ds_5ee8bf93-b0b4-4722-8a17-6b624a3af072') |
There was a problem hiding this comment.
why do you need to do this set_uuid?
There was a problem hiding this comment.
so the UUID is not randomly generated and VCR won't know how to handle the request
| include API::Actions::Destroy | ||
|
|
||
| def self.find_by_external_id(external_id) | ||
| all(external_id: external_id).first |
There was a problem hiding this comment.
this is bit misleading method. If you have 2 Recurly connectors, you'll be not able to retrieve one of them. In general, if you have two connectors and customers have same external_ids, then only one will be returned here :)
There was a problem hiding this comment.
Agreed, but this is not a newly introduced method. Maybe it's better to cover this one in another PR?
There was a problem hiding this comment.
Sure, just noticed it and wanted to make @ytvinay aware
promulo
force-pushed
the
dp-66-refactor-gem
branch
from
3ad41c9
to
3caebba
Compare
This PR adds support for new ChartMogul API endpoints and also move some classes outside the
Importnamespace in order to be compliant with our soon to be published new API structure. Stay tuned.The current
ChartMogul::Import::Customer,ChartMogul::Import::DataSourceandChartMogul::Import::Planwill lose theImportnamespace as we move towards a simpler API structure. For now, no changes are made toEnrichmentandMetricsnamespaces.Along with that, some new endpoints will now have support: