Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Fault Tolerance within the Bulk Assignments #16

Open
timcortesi opened this issue Dec 26, 2020 · 3 comments
Open

Add Fault Tolerance within the Bulk Assignments #16

timcortesi opened this issue Dec 26, 2020 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@timcortesi
Copy link
Member

Need to check the the assignment module actually exists before continuing with the rest of the assignments:
In /app/Console/Kernel.php around line 75:

The code is CURRENTLY like this...

$module = Module::where('id',$bulkAssignment->assignment->module_id)->with('current_version')->first();
if (is_null($module->module_version_id)) {
    // Do Nothing
    continue;
}

It should be changed to something like this...

$module = Module::where('id',$bulkAssignment->assignment->module_id)->with('current_version')->first();
if (is_null($module) || is_null($module->module_version_id)) {
    // Do Nothing
    continue;
}

If $module is null, the following error is thrown:

[2020-12-26 02:30:01] prod.ERROR: Trying to get property 'module_version_id' of non-object {"exception":"[object] (ErrorException(code: 0): Trying to get property 'module_version_id' of non-object at /var/app/current/app/Console/Kernel.php:76)

Additionally, to address any other possible issues, around each bulk assignment, we need to add a try / catch block. This should be between likes 73 and 74 (or thereabouts):

foreach($bulkAssignments as $bulkAssignment){
    // We should add a "try" block in here in case any errors are thrown within the bulk assignment
    if(Carbon::parse($bulkAssignment->assignment->later_assignment_date)->isToday()){

Additionally, we need to make sure that "Module" is a required field on the bulk assignment form. (You MUST select a module, or the form should not validate):

Screen Shot 2020-12-26 at 8 31 15 AM

We should also look at any other fields which might need to be required.

@timcortesi timcortesi added the bug Something isn't working label Dec 26, 2020
@timcortesi
Copy link
Member Author

Tagging @phelpsa as an FYI on this issue.

@alikemaltanriverdi
Copy link
Member

Fault prevention mechanisms have been added to the required places in Kernel.php, such as adding try/catch blocks, adding additional conditions in the if statements those are determining if an assignment should be made, and etc.
Please see the fields those were made required throughout the BComply app to prevent any additional bugs in the future:

Bulk Assignments

  1. Module To Assign when configuring a the bulk assignment.
  2. Column and Value fields those are under the AND/OR Conditional fields when configuring a the bulk assignment..
  3. Auto Assign Date, if Assign Later is selected

Groups

  1. Group Name field when creating a new group.

Group Memberships

  1. User field when adding a new member in to a group.

Module Assignments

  1. User field when adding a new assignment.
  2. Status field is now required to prevent a blank status when Marking as completed.
  3. Date fields those are visible when Date Started or Date Completed fields are selected.

Module Permissions

  1. User field is now required when providing a module privilege to a user.

Module Versions

  1. Version Name
  2. Launch URL if the version type is Articulate Tincan
  3. Youtube Code if the version type is Youtube

Reports

  1. Report Name when creating a new report
  2. Column and Value fields those are under the AND/OR Conditional fields
    1. Value field is now visible or hidden according to the values selected in column fields.
    2. It has the same conditional hide/ show as it has in the Bulk Assignment Configuration pop-up window.

Users

  1. Unique ID and Email fields are now required fields when creating a new user and updating the user attributes.

Additional tests are being made.

@alikemaltanriverdi
Copy link
Member

The most up to date code is now in the BComply environment and it seems to be working fine so far. I will post another update once we think everything is working as expected and we are ready to go live. In order for me to complete the tests, I will need to wait for Bulk Assignment to run tomorrow (at 2am or 2:30am?) as scheduled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants