Skip to content

London | 26-SDC-July | Raihan Sharif | Sprint 3 | Middleware exercises - #128

Open
RaihanSharif wants to merge 55 commits into
CodeYourFuture:mainfrom
RaihanSharif:middleware
Open

RaihanSharif wants to merge 55 commits into
CodeYourFuture:mainfrom
RaihanSharif:middleware

Conversation

@RaihanSharif

Copy link
Copy Markdown

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Task code

CYF-1165

Changelist

First write custom middleware for processing username and request body. Then use built-in body parsers. Explain why the request has to change when using built-in middleware.

Can post message, with validation.
Can get all messages.
Only return list of messages, or a single message.

Clean up js doc
Messages can now be fetched or created using REST API.

Client can then fetch subsequent messages using event streaming.
create new message handling function.
... but don't add to local messages map. That will be done by the event polling.
@RaihanSharif RaihanSharif added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 5, 2026
@Poonam-raj Poonam-raj added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 13, 2026
Comment thread built-in-middleware/app.js Outdated
Comment thread built-in-middleware/app.js
Comment thread built-in-middleware/app.js Outdated
Comment thread built-in-middleware/app.js Outdated
Comment thread custom-middleware/app.js Outdated

@Poonam-raj Poonam-raj left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd like you to remove the work you did on the chat app from this branch - make it separate, this work is solely about the middleware work

Great work otherwise, I've left some comments and then one additional thought here:

  • you have package files within each middleware folder but they are almost identical in purpose (marking express as a dependency). Can you make your package files sit higher in the repo so only one copy of those npm-related files needs to be created?

@Poonam-raj Poonam-raj added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Oct 13, 2026
@RaihanSharif

Copy link
Copy Markdown
Author

I have made all changes requested in the review.

@RaihanSharif RaihanSharif added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 14, 2026

@Poonam-raj Poonam-raj left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

fab work thank you for these changes

@Poonam-raj Poonam-raj added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants