ui: Address some Admin Partition FIXMEs (#11057)

This commit addresses some left over admin partition FIXMEs

1. Adds Partition correctly to Service Instances
2. Converts non-important 'we can do this later' FIXMEs to TODOs
3. Removes some FIXMEs that I've double checked and addressed.

Most of the remaining FIXMEs I'm waiting on responses to questions from
the consul core folks for. I'll address those in a separate PR.

ui/packages/consul-ui/app/adapters/partition.js

@@ -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 }}
@@ -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');

ui/packages/consul-ui/app/components/auth-form/index.hbs

@@ -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">

ui/packages/consul-ui/app/components/data-source/index.js

@@ -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
@@ -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' &&
@@ -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([]);
     });
   }

ui/packages/consul-ui/app/components/hashicorp-consul/index.hbs

@@ -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'}}

ui/packages/consul-ui/app/components/token-source/index.js

@@ -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);
     },
   },

ui/packages/consul-ui/app/models/service-instance.js

@@ -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;

ui/packages/consul-ui/app/routing/route.js

@@ -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 (

ui/packages/consul-ui/app/services/data-sink/protocols/http.js

@@ -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];

ui/packages/consul-ui/app/services/repository/dc.js

@@ -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;

ui/packages/consul-ui/app/templates/dc/services/show/intentions/index.hbs

@@ -56,7 +56,6 @@ as |route|>
           @filter={{filters}}
         />
   {{/if}}
-        {{!FIXME: Check param order }}
         <DataWriter
           @sink={{uri
             '/${partition}/${dc}/${nspace}/intention/'