Closed dhovart closed 2 years ago
@katjaq @r03ert0 Feel free to review and merge if ok with you!
Merging #312 (8f17688) into master (5986b7a) will increase coverage by
1.45%
. The diff coverage is88.88%
.
@@ Coverage Diff @@
## master #312 +/- ##
==========================================
+ Coverage 60.92% 62.38% +1.45%
==========================================
Files 17 16 -1
Lines 2577 2334 -243
==========================================
- Hits 1570 1456 -114
+ Misses 1007 878 -129
Flag | Coverage Δ | |
---|---|---|
integration | 53.51% <88.88%> (+0.69%) |
:arrow_up: |
unittests | 43.74% <42.22%> (+1.33%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
controller/mri/mri.controller.js | 72.08% <72.72%> (+0.15%) |
:arrow_up: |
controller/dataSlices/dataSlices.js | 81.20% <77.77%> (ø) |
|
controller/project/project.controller.js | 86.26% <100.00%> (+0.76%) |
:arrow_up: |
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 5986b7a...8f17688. Read the comment docs.
Hey! that looks fatastic! I'd like to ask just a few questions for clarification :
loggedUser
as input --> unlogged should be handled as anyone. loggedUser
variable)Thank you so much! 😊
Thank you for walking me through! Fantastic! Merged! :)
As suggested on Mattermost:
The idea here is rather to extend core classes from NeuroWebLab than to patch existing objects. Like how we have
BrainboxAccessControlService
extendingAccessControlService
.Integration tests pass but I still need to add a few test cases (and to correct a possible regression)