feat(slack): Introduce slack support channel selector

[caseyhebebrand]

A user can select a Slack support channel when creating an application, or editing an application. This will render in Application Attributes as a link to the channel. This is an optional field which is configured by (1) enabling the feature in deck settings and (2) providing a slack token to gate.

[christopherthielen]

nice work. Let me know if you want to discuss the testing comment

[christopherthielen]

i'm curious why you chose to use a class component over a hooks component??

[caseyhebebrand]

My main hesitation was ensuring that the API was only called one time, rather than after each render. I can refactor to a functional component with useEffect & a second parameter.

[christopherthielen]

If you refactor to functional component, use the useData or useLatestPromise hook!

[christopherthielen]

Can this be migrated to <ReactSelectInput mode="VIRTUALIZED" />? I'd like to move towards using a more opinionated set of components across the board.

[caseyhebebrand]

Yes, did not see this component so I will swap it out.

[christopherthielen]

this test isn't really asserting anything useful. I can replace SlackReader reference with an empty stub and the test still passes:

it('should fetch a list of public Slack channels', () => {
    const foo = { getChannels() {} };
    spyOn(foo, 'getChannels').and.returnValue(Promise.resolve(mockSlackChannels));

    foo.getChannels().then((channels: any[]) => {
      expect(foo.getChannels).toHaveBeenCalled();
      expect(channels.length).toEqual(2);
    });
  });

The only useful thing to assert that I can think of is if API.one got called with slack/channels.

[caseyhebebrand]

Ok I will think about this more. I was trying to remove the dependency on the angular mocks. I assumed this would still call the API and on a successful response return the values, while on a failed response the test would fail.

[christopherthielen]

you should be able to do something like:

    const oneSpy = spyOn(API as any, 'one').and.callThrough();
    const getSpy = spyOn(API as any, 'getFn').and.returnValue(Promise.resolve(mockSlackChannels));

then you should be able to

expect(oneSpy).toHaveBeenCalledWith('slack/channels);

... I think

[caseyhebebrand]

@christopherthielen This was the best I could do using callThrough because I cannot spy on any of the private methods. I think it is valuable enough of a test to show that we have called API correctly, while still controlling the response.

Another option is to utilize angular mocks like the other services, but I would prefer to reduce the dependency on angular as much as possible.

Another

[christopherthielen]

Agreed that we should avoid using angular mocks in any new code. this looks great to me, thanks for updating it!

[christopherthielen]

Agreed that we should avoid using angular mocks in any new code. this looks great to me, thanks for updating it!