Skip to content

created "post tweet" API endpoint - #26

Open
gid-code wants to merge 3 commits into
imtumbleweed:masterfrom
gid-code:master
Open

created "post tweet" API endpoint#26
gid-code wants to merge 3 commits into
imtumbleweed:masterfrom
gid-code:master

Conversation

@gid-code

Copy link
Copy Markdown

Created post tweet API endpoint @javascriptteacher

Comment thread index.html
payload = {
id: 1,
payload.message: "hello" } */
user_id: 1,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why the values are hardcoded?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That was already written. I just changed the name of the parameters.

Comment thread index.html
<input
type="button"
onclick="User.tweet({user:'felix',message:'hello there, this is my tweet.'})"
onclick="User.tweet({user_id:4,message:'hello there, this is my tweet.'})"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

onclick should call a function, which in turns post a tweet.

Comment thread module/api/api.js Outdated
database.connection.query(q,
(error, results) =>{ //Check if tweet already exist
if(error) throw error;
let result = results[0];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this variable may not be needed.

Comment thread module/api/api.js Outdated
(error, results) =>{ //Check if tweet already exist
if(error) throw error;
let result = results[0];
if (results && results.length != 0 && result.message == payload.message)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

results[0].messages

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll change that.

@sanjaypradeep sanjaypradeep left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor code tweak required.

@imtumbleweed

Copy link
Copy Markdown
Owner

Are we ready to merge @sanjaypradeep

@sanjaypradeep

Copy link
Copy Markdown
Collaborator

Review comments aren't addressed completely. I can give my heads-up once done!

Apologize for late intimation @javascriptteacher

@gid-code

Copy link
Copy Markdown
Author

@sanjaypradeep can you point out the comments that are not addressed completely.

Comment thread module/api/api.js Outdated
if (results && results.length != 0 && results[0].message == payload.message)
resolve(`{"success": false, "message": "tweet already exist"}`);
else {
// console.log(payload)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove this line please.

Comment thread module/api/api.js Outdated
function action_post_tweet(request, payload) {
return new Promise((resolve, reject) => {
let q = ""; // query
if (!request || !request.headers || !payload)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Actually, we wanna throw error early if validation fails here, Isn't it?

Then I prefer doing it as,

function action_post_tweet(request, payload) {
    if ( !request || !payload) {
            throw error("Some error with proper messages")
    }
    // can return from promise here.. 
    return new Promise( .. )
}```

Comment thread module/api/api.js Outdated
resolve(`{"success": false, "message": "tweet already exist"}`);
else {
// console.log(payload)
let fields = "( `user_id`, `message`, `timestamp` )";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From Line 371 to 379 - code which are available could be in a separate method saveTweet , consume that method here. Code may look clear.

@sanjaypradeep

Copy link
Copy Markdown
Collaborator

Hi @gid-code Thank for spending time in implementing this. I'm trying to help this PR to make more clear. Have given some more suggestion. Please take look, would love to have discussion if required.

@gid-code

gid-code commented Dec 19, 2019

Copy link
Copy Markdown
Author

Ok. Thank you. I will make the various changes

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