Skip to content
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

adding event functions #2

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

adding event functions #2

wants to merge 3 commits into from

Conversation

gogarattan
Copy link
Collaborator

Signed-off-by: Gaurav [email protected]
Signed-off-by: Ankur Charan [email protected]
Signed-off-by: Vaibhav
Signed-off-by: Harshita

let startTime = req.body.startTime;
let endTime = req.body.endTime;
let eventDescription = req.body.eventDescription;

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make generic body design of event data. Many new fields can be introduced later on. So accept form data as json and parse it here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okkk

endTime : endTime
})
.then((snapshot) => {
console.log('done')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modify the success message to "Added {event} to timeline successfully".

console.log('done')
}).catch((err) => {
res.send(err)
})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appropriate error statement: Error occurred while adding to timeline

eventDescription : eventDescription,
startTime : startTime,
endTime : endTime
}).then((snapshot) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to generic implementation

}).catch((err) => {
return res.send(err)
})
})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appropriate log messages.


const db = admin.database().ref();

exports.addEvent = functions.https.onRequest((req,res) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No comments? Please explain this function in comments

{
var obj = {};
obj.name = i;
data.categories.push(obj);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Response should be like :
{
"categories": ["a", "b"]
}

{
var obj = {};
obj.category = category,
obj.name = database[category][event].name;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this as previous: {
"managerial": ["a", "b"],
"programming" : ["a","b"]
}

}
else if(req.query.category == 'one')
{
let cat = req.query.eventCategory;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cat? change it

})
}
else {
return res.send("Invalid parameters.");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Print parameter also.


exports.getEventDescription = functions.https.onRequest((req,res) => {

if(req.query.events == 'all')
Copy link
Owner

@devgrpnitkkr devgrpnitkkr Sep 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this. Dead code. Never all events are required. Use cases are:

  1. Single event
  2. All events for one category.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okok


exports.getCategories = functions.https.onRequest((req,res) => {

return db.child('events').once('value')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not hard-code these strings. Declare global variables for references.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to declare them global . this is the general uniform format of writing code .


if(req.query.category == 'all')
{
return db.child('events').once('value')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not hard-code these strings. Declare global variables for references.

else if(req.query.category == 'one')
{
let cat = req.query.eventCategory;
return db.child(`events/${cat}`).once('value')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not hard-code these strings. Declare global variables for references.


if(req.query.events == 'all')
{
return db.child('eventDescription').once('value')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not hard-code these strings. Declare global variables for references.


var events=database[category];

for(var event in events)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build separate function for this

data.eventDesciption.push(obj);
}
}
return res.send(data);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no processing required

let eventName = req.query.eventName

if(eventCategory == null || eventName == null) {
res.send("Insufficient parameters.")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Print event category and event name values

db.child(`eventDescription/${eventCategory}/${eventName}`).once('value')
.then((snapshot) => {
if(snapshot == null) {
return res.send("Event Doesn't Exist.");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

print values

return res.send(err)
})
}
else if(req.query.events == 'cat')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cat? and hardcoded?

let categoryName = req.query.eventCategory;
if(categoryName == null)
{
return res.send("Insufficient Parameters.");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change error message.

Copy link
Owner

@devgrpnitkkr devgrpnitkkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modify as per comments

response.send("Invalid Parameters.");
}

const request = require('request');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

require statements on top.

{
return console.log(err);
}
console.log(body);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to log complete body?

}

let email1 = body.emails[0].value;
let email = email1.replace(/\./g,',');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make seperate function for this and rename this variable.

let node = "userRegistrations/"+ eventName;
console.log(node);
// let db = database.ref();
let db = admin.database().ref();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove


db.child(`${node}`).push(email);

db.child("users/" + email + "/events").once('value')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

registeredEvents

}
else
{
newEventString = eventString + ", " + eventName;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove space

"events": newEventString
})
.then(() => {
response.send("Registered");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return json

})
.catch((error) => {
console.log(error);
response.send("Could not Register!");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants