-
Notifications
You must be signed in to change notification settings - Fork 774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
User Scope web App Nodejs #1312
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix all comments.
MicrosoftAppPassword = "<<MicrosoftAppPassword>>" | ||
MicrosoftAppTenantId = "<<MicrosoftAppTenantId>>" | ||
AllowedHosts = "*" | ||
notificationUrl= "<<BaseUrl>>/api/notifications" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid duplicate entry, can you please use relative path for notificationUrl. It should be "/api/notifications". Internally you can add BaseUrl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so you want me to add only this "/api/notifications" right ?, I have updated please check it once and provide your feedback comment.
samples/user-scope-web-application/nodejs/Images/2.SelectAccountForLogin.png
Outdated
Show resolved
Hide resolved
samples/user-scope-web-application/nodejs/Images/3.LoginSuccess.png
Outdated
Show resolved
Hide resolved
samples/user-scope-web-application/nodejs/Images/UserScopeWebApp.gif
Outdated
Show resolved
Hide resolved
/** Creates Subscription for Team */ | ||
const createTeamAsync = async (req, res) => { | ||
var teamId = req.query.teamId; | ||
var subsId = "2"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this hardcoded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
/** Creates Subscription for UserSCope Chats */ | ||
const subscribeToSpecificChat = async (req, res) => { | ||
var chatId = req.query.chatId; | ||
var subsId = "3"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove hardcodings from everywhere, and from where we are using subsId?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
* @param {pageId}. Team and channel subscription. | ||
*/ | ||
static async createSubscription(id, token, subsId) { | ||
console.log(new Date(Date.now() + 3600000).toISOString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove un-necessary console logs from everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
let changeType = ""; | ||
|
||
switch (subsId) { | ||
case '1': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of numbering, we can use proper text for understanding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, updated In all places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks correct, Approving!
This is a sample application which demonstrates use of Team/Channel and User Scope Graph subscription that will post notifications when user create/edit/delete team/channel. | ||
|
||
## Included Features | ||
* Tabs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there tabs in this feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated..
npm install | ||
``` | ||
|
||
Run your app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this section, would you mind putting more details on how actually to run the demo?
I recall there were two parts, server + client, and you needed to run some command from VSCode directly. Those details would make it clear how to run the demo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok sure.
|
||
// Fetch teams group chat list | ||
const fetchTeamsLeftRail = async (userID, token) => { | ||
var resp = await axios.get(`/api/changeNotification/getAllChats?userId=${userID}&token=${token}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious what this API is? I have never seen this before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…com/OfficeDev/Microsoft-Teams-Samples into v-mfurquan/UserScope-WebApplication
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Good, Approving!
Executed build pipeline: |
Executed Build Pipeline |
No description provided.