Minor issues labs 1-5
Closed this issue · 1 comments
Now A01-A03, B01-B04, and "ConfigurableTab" labs.
These are from Waldek and some but not all have been addressed in the update. Need to review and check.
(Thanks @waldekmastykarz)
More issues from @waldekmastykarz
pointed out that azure-ad-jwt hasn't been updated in 4 years; want to find the best approach (Passport?)
For a GET request you don't need to set the content-type header:
TeamsAppCamp1/A01-Start-AAD/server/identityService.js
Line 130 in 8c00ca5
"Content-Type": "application/json",
Also, to be compliant with HTTP/2, all request headers' names should be lowercase (not necessary for Graph at this time but best practice)
In lab A03, you mention to update devDependencies in package.json to add adm-zip, but if you have a lockfile (which is a default these days) if you run npm i, the changes won't be picked up because npm will only install packages from the lockfile, so it would be better to say: run npm i -D adm-zip
Here: https://github.com/OfficeDev/TeamsAppCamp1/blob/main/Labs/Labs-A/Lab-A03.md#step-1-add-a-module-with-teams-helper-functions, the inTeams function is async but there's no need for async really, unless you're going to modify it later on and want to keep the number of steps lower
RE: manifest versions: JS advocates pointed out that it would make sense to maintain the version in package.json and then sync it to the manifest. It's a small detail, but something that would feel natural to JS devs and also allow them to maintain the app version in just one place
Lab 5, Step 2: in the code sample you could skip the if (employee) { check because just above you verified the opposite and redirect to a different page if it's the case, so this extra check is unnecessary:
I just noticed that you didn't commit lock files to the repo. To ensure that this code will run in the future, it would be good to include them. Otherwise, if one of the dependency changes, it could break the whole sample
Moved to new repo