Skip to content

(fix) Removes JSON.stringify from data in ajax req#378

Open
nlokare wants to merge 2 commits into
gilded-email:masterfrom
nlokare:master
Open

(fix) Removes JSON.stringify from data in ajax req#378
nlokare wants to merge 2 commits into
gilded-email:masterfrom
nlokare:master

Conversation

@nlokare

@nlokare nlokare commented Mar 10, 2015

Copy link
Copy Markdown
Member

No description provided.

@derekrabindran

Copy link
Copy Markdown

🐒 CodeMonkey Review Summary

4 findings across 1 cycle:

🔵 MINOR (2)

  • Missing input validation on client sideviews/contact-us.jade:28 — The new name and sender (email) inputs are submitted without any client-side validation. While server-side validation is the source of truth, adding required attributes and using HTML5 validation would improve UX by preventing empty submissions.
    • 💡 Add required attributes to the inputs, e.g. input#name.form-control(type='text', required, placeholder='Enter your name').
  • Removing JSON.stringify changes content type and server expectationsviews/contact-us.jade:36 — Switching from JSON.stringify(data) to data: data causes jQuery to URL-encode the payload as application/x-www-form-urlencoded instead of sending JSON. This only works if the server's /contact endpoint parses form-encoded bodies (e.g. via body-parser.urlencoded). Verify the server's middleware matches—if it expects application/json, the payload will arrive empty. Given the PR title indicates this is the intended fix, the server likely uses urlencoded parsing, but worth confirming.
    • 💡 Confirm the server uses bodyParser.urlencoded({ extended: true }) for /contact. If JSON is preferred, set contentType: 'application/json' alongside JSON.stringify(data).

💬 NIT (2)

  • Inconsistent indentation in inline scriptviews/contact-us.jade:22 — The inline JavaScript inside the script block is not indented consistently with the surrounding Jade structure, which makes the code harder to read.
    • 💡 Indent the JS body two spaces under the script tag for clarity.
  • Indentation fix is a good cleanupviews/privacypolicy.jade:8 — Dedenting the privacy policy content so the h3 headings render as siblings (rather than being nested under the previous paragraph due to over-indentation) is a correct fix. No action needed—just noting the change is appropriate.

Review cost: $0.7152

🤖 Generated by CodeMonkey

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants