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

show users that unenrolled prior to the allocation in a seperate row #199

Closed

Conversation

sevenseas-gists
Copy link

@sevenseas-gists sevenseas-gists commented Jul 31, 2020

closes #166

@codecov-commenter
Copy link

codecov-commenter commented Jul 31, 2020

Codecov Report

Merging #199 into master will decrease coverage by 0.16%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #199      +/-   ##
============================================
- Coverage     25.74%   25.58%   -0.17%     
- Complexity      767      772       +5     
============================================
  Files            43       43              
  Lines          3247     3268      +21     
============================================
  Hits            836      836              
- Misses         2411     2432      +21     
Impacted Files Coverage Δ Complexity Δ
classes/allocations_table.php 0.00% <0.00%> (ø) 35.00 <0.00> (+5.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5069214...a46d5e7. Read the comment docs.

@@ -160,17 +160,42 @@ public function build_table_by_sql() {
$noallocation->choicetitle = get_string(
'allocations_table_noallocation',
ratingallocate_MOD_NAME);
$enrolled_user_ids = array_map(function($x) {return $x->id;},
$this->ratingallocate->get_raters_in_course());
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of $this->ratingallocate->get_raters_in_course(), you could use $users

if (object_property_exists($noallocation, 'users')) {
$noallocation->users .= ', ';
} else {
$noallocation->users = '';
}
$noallocation->users .= $this->get_user_link($user);
unset($userwithrating[$userid]);
}
$data []= $noallocation;
Copy link
Contributor

Choose a reason for hiding this comment

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

If there are no unallocated users, $noallocation->users will be undefined. You could put all userlinks for noallocation in an array, and set $noallocation->users = implode(', ', $noallocationusers) afterwards (or do it the same way as around line 151).
Same goes for $unenrolled.

It looks like the problem has already existed before this PR, but it seems a good opportunity to fix it :)

@Laur0r Laur0r closed this Apr 18, 2024
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.

Allocation overview should display that a student is not enrolled anymore
4 participants