Skip to content

Conversation

@RbAvci
Copy link

@RbAvci RbAvci commented Apr 26, 2024

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Created a Mailing List REST API.
Routes
/lists - fetch all the existing list names
/lists/:name - get list by name, delete by name, add or update a list with the given name // adjusted with different status codes such as 200,201,404

Stretch 1 is done: checks if the path matches the name in JSON.

@sonarqubecloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

@Ara225 Ara225 left a comment

Choose a reason for hiding this comment

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

Good work :)


const app = express();
app.use(express.json());
app.disable("x-powered-by");
Copy link

Choose a reason for hiding this comment

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

Is there any particulaur reason for this line of code? It's a good question to ask yourself whenever you find anything odd in code.

app.use(express.json());
app.disable("x-powered-by");

const mailList = mailinglist;
Copy link

Choose a reason for hiding this comment

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

Another line that doesn't make too much sense to me - is there any particuluar reason that the var needs reassigned?

app.get("/lists", (req, res) => {
const listsArray = Object.keys(mailList);
res.status(200).json(listsArray);
});
Copy link

Choose a reason for hiding this comment

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

Good work, appreciate the comments - makes it easier to skim read

} else {
res.status(404).json({ error: "List not found" });
}
});
Copy link

Choose a reason for hiding this comment

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

👍

} else {
res.status(404).json({ message: "Name not found for delete" });
}
});
Copy link

Choose a reason for hiding this comment

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

👍

mailList[name] = members;
res.status(201).json({ message: "New list created successfully." });
}
});
Copy link

Choose a reason for hiding this comment

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

Amazing, nice to see you've done the stretch goal as well!

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.

2 participants