Enhancement/40 add outlook location field

[Philldomd]

Pull Request Description

Sets the Location field to a variable that is configured in the config.js file. This will remove the question on send that blocks, asking if the user wants to send a message without location

Fixes #40

Checklist

• [x] Changes are limited to a single goal (avoid scope creep)
• [x] I confirm that I have read any Contribution and Development guidelines (CONTRIBUTING and DEVELOPMENT) and are following their suggestions.
• [x] I confirm that I wrote and/or have the right to submit the contents of my Pull Request, by agreeing to the Developer Certificate of Origin, (adding a 'sign-off' to my commits).

[Philldomd]

Thanks again for your contribution, the feature works as expected! 😁

But I have added some comments that I think should be adressed or discussed. Also, I was not able to run the tests locally. They seem to fail for the OfficeCallHandler.test.ts file

What problem did you get here? Was npm install run to get latest changes in package?

[Ayko1595]

@Philldomd Yes I have the latest version of the code and also ran npm install. Here is the error message I received:

[Philldomd]

@Ayko1595 If you change to the following will that work, in test/OfficeCallHandler.test.ts? 10 import { OfficeMockObject } from "office-addin-mock"; 35 const setLocationMock = new OfficeMockObject(mockSetLocationServer) as any; 69 const setDataMock = new OfficeMockObject(mockSetDataServer) as any;

does the test work then?

[Ayko1595]

@Philldomd It does not work after those changes, I am still getting those errors. I think the issue could be the testing environment. The jest configuration specifies a jsdom environment, which is used on client-side testing. The tests you want to run in the OfficeCallHandler relies on a node environment which causes the test to fail. There is probably a mismatch in the configurations and the test files

[Philldomd]

I changed to Node instead and all other tests failed. I get everything to pass, both on my workstation and my laptop. So i can't reproduce this. I have removed package-lock and node_modules and run install again with the same resault. Have you added office-addin-mock?

[Ayko1595]

Office-addin-mock is installed yes. I am running this on MacOS and it still fails unfortunately. If you fix the comment regarding the Jest config's license header we will be able to run the GitHub action that tests the code

[janderssonse]

Off-topic: Nice to see you at the meeting @Philldomd and while I myself have 0% time left this week to help out reviewing @Ayko1595 is doing a super job here though, so no need for me to chip in anyway), I'm very glad to see these PR's arrive to the project, and that FK is part of this. Much cred to all involved in this. This is collaboration for real.

[Philldomd]

Code is squashed and updated as requested

[Philldomd]

Testing with older package.json file

[Philldomd]

I found out now from a co-worker that this actually introduces a bug, the location field is cleared from other information upon generating a jitsi link. I will have a look into i and ensure that this does not happen here. Await my next commit for that fix and a testcase that checks it.

[Ayko1595]

@Philldomd Oh okay, well nice catch by our co-worker. If it is possible to introduce a unit test that tests this issue it would be great :-)

[Philldomd]

I will make sure to implement a test for it yes

[Philldomd]

Okey now I think this one is finally finished. Added a lot of tests to control the location field. At least it is not duplicating the text or removing other location field information.

[Ayko1595]

Looks good @Philldomd but there seems to be a conflict in the commands.ts file

[Philldomd]

Let's wait for bugfix and then merge this code in. There will be some major changes when that happens