Get At-A-Glance year list from data

[Brian Lewis]

Follows from 46.

Lookups now includes LookupList 'surveyYears' in the cache Use this to populate the availableyears list:

strategy - inject Lookups service into the controller. Expose the Lookups service as a public method of the controller Refer to Lookups.cache["surveyYears"] as the source of the options in the <select>

Lists in the lookup cache are of type ILookupList, and array of ILookupEntry with some extra searching methods ( see LookupsService.ts for defn)

In particular, each entry has properties C: (code) and N: ( name or description associated to that code) for a <select>, you'll use C as the value and N as the display. In some cases, these return the same value (e.g. surveyYears C and N are just the year number.)

surveyYears sorts DESC, I think its better to have the most recent data at the top of the list.

[Brian Lewis]

Further note on Lookups: the init() method initialises the core set of lists in the cache. To see what these are, look in Pineapples.Data DSlookups class.

init is called in a resolve block in ui-router. this means that the routes are not loaded until the promise resolves. In other words, you can always assume that the core lookups are populated.

[Ghislain Hachey]

I'll do this as soon as work from #67 is merged since the work will be directly in the new AtAGlance typescript controller.

[Ghislain Hachey]

OK. I've tried your strategy but could not really make it work properly without adding a fair bit more code. For example, when using the ILookupList in select option I have to work with ILookupEntry instead of plain numbers. This would be fine but as a result I have to change types of all related variable (baseYear, selectedYear getters/setters, etc.), this is also fine but then the simple calculation in the setters can no longer work with plain number arithmetic so I think ILookupEntry would have to be specialised with an add operation?!. At that point I took the easy way and only retrieved ILookupList's ILookupEntry.N to work with that. Seems to work.

I also started doing a bit of documentation in the code as I learn and make use of to make it more clear to others using it, I hope you will not mind :)

I've pushed what I have if you can have a look when you can and let me know if you think it's fine for a PR.

[Brian Lewis]

oh wow I see what you mean.

ng-options can get very complicated, and every time I use it I feel like i'm starting from scratch - but, there's generally a way to get what you want.

this link goes through the options.

There are shorthand forms, but always using the most elaborate means you've only got one thing to remember:

<bound-value> as <label> for <object variable> in <array>

I think all you need here is

ng-options="lkp.C as lkp.N for lkp in vm.Lookups.cache['selectedYears']"

with this syntax lkp.C is bound to the select, not the ILookupEntry - so it's still a number. Bear in mind in this list lkp.C and lkp.N are the same, but usually C is a code and N is the name.

That should be all it takes - just make Lookups public in the controller. You won't need $q; and you can be confident that the Lookups are already initialised by the Resolve in ui-router.

[Ghislain Hachey]

Like I said, I went that path. In fact, I needed:

ng-options="lkp.N for lkp in vm.Lookups.cache['selectedYears'] as track by lkp.C"

The thing is the ng-models (baseYear, selectedYear) on the selects then become ILookupEntry adding IMO unnecessary bloat code because then you need to change the getters/setters to work with ILookupEntry including the number arithmetic as I mentioned above. The simple this.baseYear - 1 then no longer compiles a this is a ILookupEntry - 1 which is not supported (this is why I mentioned the specialised ILookupEntry for years with extra minus one year method, etc.)

I am not sure where you saw the use of $q, I was just playing around that and did not even think I had it commited. I do simply expose vm.Lookups.cache['selectedYears'] values. Actually, I am just noticing that I forgot to remove the injected $q, I see where you got this now, though I am not using it. Was just experimenting with your service and then used the cache.

I'll remove $q and commit in a minute.

[Brian Lewis]

Your syntax using track by is not quite the same as what I have above using <select> as. track by does not affect the binding, only the identifier used internally to produce a unique identifier for the iteration - the angular documentation on this is, not unusually, very confusing.

Take a look in e.g. SchoolSearcher.cshtml where there are a number of dropdowns based on lookups.

      <div class="col-md-2-a">
          <div class="form-group">
              <label for="srchlistNames">Custom Lists</label>
              <select class="form-control" id="srchlistNames"
                      ng-model="vm.params.ListName"
                      ng-options="r.C as r.N for r in vm.lookups.cache.listNames">
                  <option value=""></option>
              </select>
          </div>
      </div>

These all bind the scalar value r.C to the select.

[Ghislain Hachey]

I just tried and it works. It's a little confusing that the form

ng-options="lkp.N for lkp in vm.Lookups.cache['selectedYears'] as track by lkp.C"

ends up binding the ng-models to a ILookupEntry while

ng-options="lkp.C as lkp.N for lkp in vm.Lookups.cache['selectedYears']"

does not. I'll push in a couple of minutes.

[Ghislain Hachey]

Pushed and hopefully ready :)

[Ghislain Hachey]

Hey. So I see you merged stuff into develop. How would you prefer I do the PR for this work?

[Brian Lewis]

I created a branch and a PR, following I think the conventions you have put in place. So - the next step - rather than me merging my own PRs! - are you happy to be a reviewer with merge permissions into develop so PRs I create are assigned to you to review?

[Brian Lewis]

Please create a PR for Issue66 and we'll get that done now.

[Brian Lewis]

Just one last tiny thing - can you run tslint and fix the three lint issues in this file - remove the two comments; add the break

[Ghislain Hachey]

Fixed lint issues, pushed and created PR. Eventually it will be good if the linting is part of _build_deploy so it must pass lint to go further. In my CSS branch the linting is enforced at build step (with only basic tests for now as you configured it).

[Ghislain Hachey]

Just created PR and there is a conflict with pineapples.ps

[Ghislain Hachey]

Regarding reviewing PRs I'm happy to give it a go. My understanding of the whole system is nowhere near yours but I suppose a second pair of eye can't hurt.

[Brian Lewis]

branch Issue66 merged

[Brian Lewis]

Noted about pineapples.js and I can see with more active branches going on this will be a pain. See #76