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

ui: Address some Admin Partition FIXMEs #11057

Merged
merged 1 commit into from
Oct 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ui/packages/consul-ui/app/adapters/partition.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import Adapter from './application';

// Blocking query support for partitions is currently disabled
export default class PartitionAdapter extends Adapter {
// FIXME: Check overall hierarchy again
async requestForQuery(request, { ns, dc, index }) {
const respond = await request`
GET /v1/partitions?${{ dc }}
Expand All @@ -12,7 +11,7 @@ export default class PartitionAdapter extends Adapter {
await respond((headers, body) => delete headers['x-consul-index']);
return respond;
}

// TODO: Not used until we do Partition CRUD
async requestForQueryRecord(request, { ns, dc, index, id }) {
if (typeof id === 'undefined') {
throw new Error('You must specify an id');
Expand Down
2 changes: 1 addition & 1 deletion ui/packages/consul-ui/app/components/auth-form/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
focus=(action 'focus')
)}}
<Guard @name="hasValue" @cond={{action 'hasValue'}} />
{{!FIXME: Call this reset or similar }}
{{!TODO: Call this reset or similar }}
<Action @name="clearError" @exec={{queue (action (mut error) undefined) (action (mut secret) undefined)}} />
<div class="auth-form" ...attributes>
<State @matches="error">
Expand Down
11 changes: 9 additions & 2 deletions ui/packages/consul-ui/app/components/data-source/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { inject as service } from '@ember/service';
import { tracked } from '@glimmer/tracking';
import { action, get } from '@ember/object';
import { schedule } from '@ember/runloop';
import { runInDebug } from '@ember/debug';

/**
* Utility function to set, but actually replace if we should replace
Expand Down Expand Up @@ -89,7 +90,8 @@ export default class DataSource extends Component {

@action
disconnect() {
// FIXME? Should we be doing this here
// TODO: Should we be doing this here? Fairly sure we should be so if this
// TODO gets old enough (6 months/ 1 year or so) feel free to remove
if (
typeof this.data !== 'undefined' &&
typeof this.data.length === 'undefined' &&
Expand Down Expand Up @@ -182,7 +184,12 @@ export default class DataSource extends Component {
this.source.readyState = 2;
this.disconnect();
schedule('afterRender', () => {
// FIXME: Lazy data-sources
// TODO: Support lazy data-sources by keeping a reference to $el
runInDebug(_ =>
console.error(
`Invalidation is only supported for non-lazy data sources. If you want to use this you should fixup support for lazy data sources`
)
);
this.connect([]);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
}}
@onchange={{action (mut this.partitions) value="data"}}
/>
{{!FIXME: Do partitions do the same as namespace deletion? }}
{{#each (reject-by 'DeletedAt' this.partitions) as |item|}}
<MenuItem
class={{if (eq @partition item.Name) 'is-active'}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export default Component.extend({
},
};
};
// FIXME: We should probably put the component into idle state
// TODO: We should probably put the component into idle state
this.onchange(e);
},
},
Expand Down
4 changes: 1 addition & 3 deletions ui/packages/consul-ui/app/models/service-instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ export default class ServiceInstance extends Model {
@alias('Service.Tags') Tags;
@alias('Service.Meta') Meta;
@alias('Service.Namespace') Namespace;

// FIXME: Is parition top level or Service.Partition?
@attr('string') Partition;
@alias('Service.Partition') Partition;

@filter('Checks.@each.Kind', (item, i, arr) => item.Kind === 'service') ServiceChecks;
@filter('Checks.@each.Kind', (item, i, arr) => item.Kind === 'node') NodeChecks;
Expand Down
5 changes: 4 additions & 1 deletion ui/packages/consul-ui/app/routing/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ export default class BaseRoute extends Route {
return value;
}

// FIXME: this is only required due to intention_id trying to do too much
// TODO: this is only required due to intention_id trying to do too much
// therefore we need to change the route parameter intention_id to just
// intention or id or similar then we can revert to only returning a model if
// we have searchProps (or a child route overwrites model)
model() {
const model = {};
if (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ export default class HttpService extends Service {
return setProperties(instance, data);
}

// TODO: Currently we don't use the other properties here So dc, nspace and
// partition, but confusingly they currently are in a different order to all
// our @dataSource uris @dataSource uses /:partition/:nspace/:dc/thing whilst
// here DataSink uses /:parition/:dc/:nspace/thing We should change DataSink
// to also use a @dataSink decorator and make sure the order of the parameters
// is the same throughout the app As it stands right now, if we do need to use
// those parameters for DataSink it will be very easy to introduce a bug due
// to this inconsistency
persist(sink, instance) {
const [, , , , model] = sink.split('/');
const repo = this[model];
Expand Down
4 changes: 2 additions & 2 deletions ui/packages/consul-ui/app/services/repository/dc.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ export default class DcService extends RepositoryService {
const items = this.store.peekAll('dc');
const item = items.findBy('Name', params.name);
if (typeof item === 'undefined') {
// FIXME: HTTPError
// TODO: We should use a HTTPError error here and remove all occurances of
// the custom shaped ember-data error throughout the app
const e = new Error('Page not found');
e.status = '404';
// FIXME: EDError
throw { errors: [e] };
}
return item;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ as |route|>
@filter={{filters}}
/>
{{/if}}
{{!FIXME: Check param order }}
<DataWriter
@sink={{uri
'/${partition}/${dc}/${nspace}/intention/'
Expand Down