Skip to content

feat(task-queue) Job scheduling with deadline reminder task#61

Open
moffatethan wants to merge 8 commits into
mainfrom
feat-be-task-queues
Open

feat(task-queue) Job scheduling with deadline reminder task#61
moffatethan wants to merge 8 commits into
mainfrom
feat-be-task-queues

Conversation

@moffatethan

@moffatethan moffatethan commented May 28, 2021

Copy link
Copy Markdown
Contributor

What this PR does (required):

  • Setups Bull
  • Runs initializing work on server startup.
  • Split job work into process files given to queue initialize in app.js
  • Plans out game plan for processing deadline reminders in the process file.

Screenshots / Videos (required):

Screen Shot 2021-06-02 at 12 06 32 PM

Any information needed to test this feature (required):

  • Setup Redis. On Mac brew install redis is my recommendation.
  • Setup environment variables or remove the options from the initializeQueue option which will automatically use localhost.
  • Right now, you manually have to add a job to the queue. This would be done via a route.

Notes

I am still planning my approach to adding the cards to the job queue. However, I am thinking adding a route that the front-end can hit that passes down the card id. By doing this, the job processor can lookup the card directly and do it's check for deadlines and process accordingly. Furthermore, I would also create routes for removing a job from the queue (i.e., a card deadline is removed).

Any issues with the current functionality (optional):

  • The functionality works, the code isn't as clean as I would like to be as we do not have multiple models and work off of one model that stores POJOs. This code could be cleaned up a lot more if we utilize models for Cards and Columns and making sure a user is attached to a Card we can just fetch all the Cards and filter with Mongoose ODM.

Comment thread server/utils/sendEmail.js Outdated
Comment thread server/utils/sendEmail.js
Comment on lines +11 to +13
to: toEmail,
from: 'ethanmoffat@hey.com',
subject: subject,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The from email usually shouldn't be the same as your personal email but I did this for testing purposes. This should be updated in a configuration file with a verified sender that you setup in your SendGrid.

Comment on lines +40 to +48
const mappedBoard = boards.map((board) => {
return {
name: board.name,
user: board.user,
cards: board.columns.map((col) =>
col.cards.filter((card) => new Date().getDay() === new Date(card.dueDate).getDay())
).flat()
}
});

@moffatethan moffatethan Jun 2, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, the way we designed our back-end this isn't the most efficient way of doing this. If we want to make this more efficient we should consider creating Card models as opposed to having everything as a a POJO.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree; this could be discussed in a future meeting on how to approach our models issue

@moffatethan moffatethan marked this pull request as ready for review June 2, 2021 19:23
@moffatethan moffatethan requested review from amahalwy, dproc96 and eng-dallavecchia and removed request for amahalwy and dproc96 June 2, 2021 19:26
@moffatethan moffatethan changed the title feat(task-queue) Initial implementation for job queues feat(task-queue) Job scheduling with deadline reminder task. Jun 2, 2021
@moffatethan moffatethan changed the title feat(task-queue) Job scheduling with deadline reminder task. feat(task-queue) Job scheduling with deadline reminder task Jun 2, 2021

@amahalwy amahalwy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left some comments but otherwise, looks good to me!

@moffatethan

Copy link
Copy Markdown
Contributor Author

Holding off on a merge here because with the plans to refactor this implementation will need to adapt to the 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.

2 participants