(rebased) Refactor sort/search/pagination for ClinicalData table endpoint #10766
Conversation
dippindots
force-pushed
the
demo-fix-clinical-table-sorting-rebase
branch
from
0efc4e7
to
5d3077f
Compare
|
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| public class PaginationCalculator { |
There was a problem hiding this comment.
This is a util class. Should not be injected. Use Static methods
|
|
||
| @Override | ||
| public List<CancerStudy> getAllStudies(String keyword, String projection, Integer pageSize, Integer pageNumber, | ||
| String sortBy, String direction) { | ||
|
|
||
| return studyMapper.getStudies(null, keyword, projection, pageSize, offsetCalculator.calculate(pageSize, pageNumber), | ||
| return studyMapper.getStudies(null, keyword, projection, pageSize, paginationCalculator.offset(pageSize, pageNumber), |
There was a problem hiding this comment.
PaginationCalculator::offset(pageSize, pageNumber)
| // Apply pagination to the sampleId list. | ||
| Integer toIndex = paginationCalculator.lastIndex(offset, pageSize, allSampleInternalIds.size()); | ||
| List<Integer> visibleSampleInternalIds = allSampleInternalIds.subList(offset, toIndex); | ||
|
|
||
| List<ClinicalData> sampleClinicalData = clinicalDataRepository.getSampleClinicalDataBySampleInternalIds( | ||
| visibleSampleInternalIds | ||
| ); | ||
| List<ClinicalData> patientClinicalData = clinicalDataRepository.getPatientClinicalDataBySampleInternalIds( | ||
| visibleSampleInternalIds | ||
| ); | ||
|
|
||
| // Merge sample and patient level clinical data and key by unique sample-key. | ||
| sampleClinicalDataCollection.setByUniqueSampleKey( | ||
| Stream.concat(sampleClinicalData.stream(), patientClinicalData.stream()) | ||
| .collect(Collectors.groupingBy(clinicalDatum -> | ||
| calculateBase64(clinicalDatum.getSampleId(), clinicalDatum.getStudyId()) | ||
| ))); |
There was a problem hiding this comment.
Just from observation. Our functions are usually bigger than they need to be. We should start focusing on keeping them small. With that, I would extract this out into a fn maybe buildSampleClinicalDataCollection
|
|
||
| import java.util.Base64; | ||
|
|
||
| public class Encoding { |
There was a problem hiding this comment.
Encoder would be a better name
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| public class SampleClinicalDataCollection { |
There was a problem hiding this comment.
Probably should start making Models not returned by MyBatis (Java Objs created by query results... Persistence layer) Immutable.
I would suggest making the class immutable (using a Builder Pattern to build the object)
|
Weren't most, if not all, of these fixed with @fuzhaoyuan PR? |
There was a problem hiding this comment.
These should not be added
|
Correct @JREastonMarks - I think this is superseded by: #10407 |
Fix # (see https://help.github.com/en/articles/closing-issues-using-keywords)
Describe changes proposed in this pull request:
Checks
Any screenshots or GIFs?
If this is a new visual feature please add a before/after screenshot or gif
here with e.g. Giphy CAPTURE or Peek
Notify reviewers
Read our Pull request merging
policy. It can help to figure out who worked on the
file before you. Please use
git blame <filename>to determine thatand notify them either through slack or by assigning them as a reviewer on the PR