Fix the request comments count #4235
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4235 +/- ##
==========================================
Coverage ? 86.79%
==========================================
Files ? 1369
Lines ? 29746
Branches ? 0
==========================================
Hits ? 25817
Misses ? 3929
Partials ? 0 ☔ View full report in Codecov by Sentry. |
|
Code Climate has analyzed commit 174c98b and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 86.7%. View more on Code Climate. |
wendyyang
changed the title
| def self.get_all_comments(request) | ||
| counts = Comment.counts_for_requests([request]) | ||
| counts[request.id] | ||
| end |
There was a problem hiding this comment.
Handy that this counts_for_requests method existed already! Looks like that's used for a similar link on the Pipeline inbox page, e.g. here - https://training.sequencescape.psd.sanger.ac.uk/pipelines/51
spec/models/request_spec.rb
Outdated
| @@ -494,4 +494,21 @@ | |||
| expect(subject[request_type2].started).to eq(1) | |||
| end | |||
| end | |||
|
|
|||
| describe '.get_all_comments' do | |||
There was a problem hiding this comment.
| describe '.get_all_comments' do | |
| describe '#get_all_comments' do |
I was going to say that this is the convention elsewhere in Sequencescape. But it looks like we've got both!
There was a problem hiding this comment.
Yes, I did see both and not sure what is the difference. if they are the same, look like using '#' is more than '.'
There was a problem hiding this comment.
'.' is for class methods. '#' is for instance methods.
There was a problem hiding this comment.
in this case, I should use '.' then?
There was a problem hiding this comment.
The signature self.get_all_comments(request) shows it is a class method. Test for this method by convention will have a '.' prefix.
There was a problem hiding this comment.
OK, thanks! will change back my initial one.
Closes #
Changes proposed in this pull request
add method in Model request to get all comments associated to the request.
replace existing one which only count the comment for the request, but not associated receptacle & labware comments in the view.
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to
main]- Check story numbers included
- Check for debug code
- Check version