Conversation
… TriggerHandler, and JobApplicationTrigger
Job application management
…statement in TaskManager
Lwc job board
Lwc paycheck calculator
…agement Automation and task management
…nagement Queueable Class added for Primary contact automation
This reverts commit 75c9218.
… on Job Application
…nagement Update Test class and TestDataFactory to take away setting Name field…
…nagement Queueable Class added for Primary contact automation
…ere-reverted Jeremys changes that were reverted
Event management
Clean up stale jobs batch
Event management
removed record types for files
changes to layouts and rec types that didnot stick
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Really great work on this project! I was happy to see the effective collaboration in GitHub and your ability to resolve merges across branches. Your code is very clean and easy to read, which made reviewing a much easier and enjoyable task for me! I think you did a great job accounting for various exception use cases and handling errors accordingly. I think the tests you included were well structured, although I would have preferred to see more coverage for each class. I think you did a great job separating out your logic to make your code more modular. Most of my comments are suggestions for further improvement and just a couple areas where the logic might need to be reviewed. Awesome job, team!
| @@ -0,0 +1,31 @@ | |||
| global class CleanUpStaleJobApplications implements Database.Batchable<SObject>, Database.Stateful { | |||
There was a problem hiding this comment.
I like the idea of having this as a global class so that if you package your application as a managed package your customers can still call to schedule/execute the batch.
| FROM Job_Application__c | ||
| WHERE Job_Application_Status__c != 'Closed' | ||
| AND Job_Application_Status__c != 'Accepted' | ||
| AND Follow_up_date__c <= :System.today().addDays(-30) |
There was a problem hiding this comment.
I think filtering on the Follow Up Date is a great alternative to creation date since some Applications might take longer than 30 days to make it through all of the stages. However, it would be good to add some automation around making sure this field gets populated.
| @isTest | ||
| private class CleanUpStaleJobApplicationsTest { | ||
| @isTest | ||
| static void testBatchJob() { |
There was a problem hiding this comment.
Its good practice to still explicitly declare your test methods 'private' even though they are by default.
| @isTest | ||
| private class CleanUpStaleJobApplicationsTest { | ||
| @isTest | ||
| static void testBatchJob() { |
There was a problem hiding this comment.
This is a good positive test case, but you should also have tests for negative and exception paths.
| for (Job_Application__c app : updatedApps) { | ||
| System.assertEquals('Closed', app.Job_Application_Status__c, 'The status should be updated to Closed for stale applications.'); | ||
| System.assertEquals('Closed by automated process', app.Notes__c, 'Notes should indicate closure by the automated process.'); | ||
| System.debug('Updated Application: ' + app.Id + ' Status: ' + app.Job_Application_Status__c); |
| // List for salary breakdown (for first section) | ||
| get salaryBreakdown() { | ||
| return [ | ||
| { label: 'Gross Monthly Salary', value: this.monthlyGrossSalary }, |
There was a problem hiding this comment.
Consider using custom labels for the hardcoded text
| @track takeHomePay = 0; | ||
|
|
||
| federalTaxBrackets = [ | ||
| { limit: 11000, rate: 0.1 }, |
There was a problem hiding this comment.
Consider leveraging custom metadata to store tax bracket info so that you can easily update on a yearly basis without needing a code change
| let tax = 0; | ||
| let remainingIncome = income; | ||
|
|
||
| for (let bracket of this.federalTaxBrackets) { |
There was a problem hiding this comment.
I think you need to adjust how the limit is subtracted for each bracket here
|
|
||
| // Calculate tax-related amounts and take-home pay | ||
| calculateTaxLiabilities() { | ||
| this.federalIncomeTax = this.calculateFederalIncomeTax(this.annualSalary).toFixed(2); |
There was a problem hiding this comment.
I would use toFixed() more sparingly and only round the final calculations
| ]; | ||
|
|
||
| handleChange(event) { | ||
| this.annualSalary = Number(event.target.value); |
There was a problem hiding this comment.
Maybe add some validation here (i.e. make sure annualSalary is always a positive number)
3 participants
👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to the default branch since the assignment started. Your teacher can see this too.
Notes for teachers
Use this PR to leave feedback. Here are some tips:
For more information about this pull request, read “Leaving assignment feedback in GitHub”.
Subscribed: @jeremymshull @Dlinebe8 @bennyone1