ui: Fix ember-can

[johncowen]

Following on from #12493

I also spotted a very weird ember-can error right at the very end or the work, and whilst the code related to ember-can in the changeset here gets things working without a big read unfathomable console error - it doesn't read well: is license should be can license and use ember-can directly. I've briefly looked into this and converting the Helpers here to native classes helped, but I still get the error when using can. I quickly tried an upgrade but that unfortunately breaks things differently - all in all I wanted to keep this PR as clean as possible. I'll continue to look at this ontop in a separate PR as its already growing in size due to this ember-can problem.

This is the error that was presenting when you where on the new Overview route and you used the DC menu to switch DCs:

I tried a few things in the above PR before realizing it was becoming a bit of a rabbit hole, so I decided to punt the problem in the above PR with a fix that didn't read well and didn't really make sense, in order to make a cleaner ember-can only related PR here.

Using native class syntax seemed to help somewhat in the above PR, so I tried upgrading to the latest 4.1.0 of ember-can, this then presented more problems (which is where I felt like I was on my way into the rabbit hole):

The fix:

1. Upgraded ember-can. ember-can 'deprecated' using the can service and changed the name of the class/service to be abilities aswell as the property used for any property injection within ember-can, so then...
2. I fixed up both any references to ember-can's can (👯‍♂️) in our codebase, and also updated any references to the can service to use abilities instead. There are places where we use @service('abilities') can and therefore this.can.can() in our codebase (👯), this isn't an issue for us, its just the name of the property we use to refer to the renamed can class/service.
3. Unfortunately we then got an error saying that parse wasn't available on abilities, which was strange as this was entirely vendor code. From past experience I've noticed that ember can be problematic if you name anything service (or namespace or ns) which as you can imagine is a good gotcha for folks working with Consul and Ember:

consul/ui/packages/consul-ui/app/instance-initializers/container.js

Lines 50 to 54 in 16085d7

	// 'service' service is not returned by catalogEntriesByType, possibly
	// related to pods and the service being called 'service':
	// https://github.com/ember-cli/ember-resolver/blob/c07287af17766bfd3acf390f867fea17686f77d2/addon/resolvers/classic/container-debug-adapter.js#L80
	// so push it on the end
	repositories.push('repository/service');

It seemed to be trying to use our ServiceAbility as the AbilitiesService and therefore parse didn't exist on that class. So I tried changing our abilities/service.js file to be called abilities/zervice.js, luckily we have a nice little entrypoint already in our app where we can hide this from the end user (you'll see that in the changeset). This fixed that issue 🎉 🕺

4. Finally we extended ember-can's can helper (💃) with a little isDestroyed guard which prevents the bug

👯‍♂️💃🎉🕺👯

Not so happy that its less likely we can write this.can.can() though 😆

Oh lastly the ember-can upgrade brought along a bunch of stuff with it including electron-to-chromium. I've no idea why 🤷 . But using yarn why tells me its all ember-can upgrade related.

P.S. Ideally the service > zervice thing is temporary, but its less horrible than the is license thing, and moreover isn't end user/template-engineer facing

[natmegs]

FWIW - I hit this same error and opened this issue minutebase/ember-can#156 in the ember-can repo. I verified that by using the changes from the latest commits (not included in latest release) - this problem goes away. Depending on how urgent this is for you, maybe it would be worth waiting?

[johncowen]

Hey! 🎉 Thanks for reviewing.

Oh I didn't see that issue! Thanks for passing that over. That's encouraging to know that once that issue has been fixed upstream we can just upgrade and delete our helper/can.js file.

Apart from that though, I don't think that would fix this issue:

So I tried changing our abilities/service.js file to be called abilities/zervice.js, luckily we have a nice little entrypoint already in our app where we can hide this from the end user (you'll see that in the changeset). This fixed that issue 🎉 🕺

So most of the code in this PR would be required even if the issue linked to was fixed. If it is fixed by then, we can just move ze zervice file back to ze service file, this is why it was handy that we had a single entry point hidden away from our templates.

Urgency-wise I'd say on the 5-6 week timeline we have for build of a rather large feature. We are roughly half way through the timeline with about 10% of the work done (about 2% reviewed/merged) 😅 . You can have a look at what is to be built here https://www.figma.com/file/qlD6mJxNCtDvy4LarWWqHL/Overview?version-id=1610838239&node-id=1365%3A5306

[johncowen]

@natmegs I also just spotted this open PR from April 2021, so almost a full year ago.

minutebase/ember-can#132

which I think is the same problem we are both seeing.

Taking this into account I would definitely like to merge this without waiting on the ember addon.

Is there anything else blocking approval here or is there anything else you would like me to address, or any questions or queries on anything here? Lemme know if so

I know it doesn't require an approval as its a chained PR, but I mostly prefer to get an approval on every single one of my PRs - chained or otherwise.

Bit of a sidenote, but I also spotted this:

minutebase/ember-can#56

which is something we added to the Consul UI codebase a while back, interesting to see other people have a similar need.

[natmegs]

sounds good. all the observer stuff that is the root of the error in the PR desc has been removed now in the main branch minutebase/ember-can@a14a0ce, it just wasn't included in latest release. next release should solve that issue and minutebase/ember-can#132. but will ✅ and unblock ya!

[amyrlam]

Just checked in with @natmegs on Slack on her prior comments and confirmed it's cool to +1 this. Hopefully ember-can can get all the fixes in upstream down the road