Pivotal ID # 177267343: Validate EU-ToxRisk Submissions #352

jhoanmanuelms · 2021-03-16T15:46:44Z

https://www.pivotaltracker.com/story/show/177267343

Implement collection validators
Update the submission submitter to use them

Delivers # 177267343

- Implement collection validators - Update the submission submitter to use them

Juan-EBI · 2021-03-17T11:15:19Z

...commons-model-extended-mapping/src/main/kotlin/ebi/ac/uk/extended/mapping/to/ToSubmission.kt

@@ -18,11 +22,14 @@ fun ExtSubmission.toSimpleSubmission(): Submission = Submission(
 )

 private fun ExtSubmission.getSubmissionAttributes(): List<Attribute> {
-    val subAttrs = attributes.map { it.toAttribute() }.toMutableSet()
+    val subAttrs =


why are we filtering it here? I was expecting when transforming to simple submission

This method comes from toSimpleSubmission (check line 16)

- Code review comments - Unit and integration testing

Juan-EBI · 2021-03-20T13:26:28Z

...commons-model-extended-mapping/src/main/kotlin/ebi/ac/uk/extended/mapping/to/ToSubmission.kt

@@ -18,11 +21,14 @@ fun ExtSubmission.toSimpleSubmission(): Submission = Submission(
 )

 private fun ExtSubmission.getSubmissionAttributes(): List<Attribute> {
-    val subAttrs = attributes.map { it.toAttribute() }.toMutableSet()
+    val subAttrs =
+        attributes


I think we should create a private method getpageTabAttributes to make clear why we remove the validator.

Juan-EBI · 2021-03-20T13:27:36Z

commons/commons-test/src/main/kotlin/ebi/ac/uk/test/ExtSubmissionFactory.kt

+import java.time.OffsetDateTime
+import java.time.ZoneOffset.UTC
+
+val basicExtSubmission = ExtSubmission(


we have create many of this, eventually we will need to put in common and use the same one

That's actually what I did with this change. I placed this one in commons-test and used it from everywhere else

Juan-EBI · 2021-03-20T13:30:15Z

submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/service/ParentInfoService.kt

+
+    private fun collectionInfo(accNo: String) = queryService.getBasicCollection(accNo)
+
+    private fun validate(validator: String, submission: ExtSubmission) =


this method is not doing what is said that is does making logic harder to follow, it does not validate but it gets the validation bean and validate (it is also doing to two operations and not a single one)

Juan-EBI · 2021-03-20T13:31:42Z

...r/src/test/kotlin/ac/uk/ebi/biostd/submission/validator/collection/EuToxRiskValidatorTest.kt

+        val requestSlot = slot<HttpEntity<FileSystemResource>>()
+
+        every {
+            restTemplate.postForObject(testUrl, capture(requestSlot), EuToxRiskValidatorResponse::class.java)


I think we can use .postForObject<EuToxRiskValidatorResponse can not we? as in the normal code.

Juan-EBI · 2021-03-20T13:34:21Z

...r/src/test/kotlin/ac/uk/ebi/biostd/submission/validator/collection/EuToxRiskValidatorTest.kt

+    }
+
+    private fun setUpRestTemplate(response: EuToxRiskValidatorResponse): CapturingSlot<HttpEntity<FileSystemResource>> {
+        val requestSlot = slot<HttpEntity<FileSystemResource>>()


IMO we should declare this as a field and in each test:

every { restTemplate.postForObject<>(testUrl, capture(requestSlot)) } } returns response

I think way when one is reviewing the test does not need to check what setUpRestTemplate does as the setup is part of each test. Also I think we do not need type I think compiler with infer it.

Juan-EBI

LGTM please check comments before merge

Code review comments

Feature * Pivotal ID # 176052254: JSON Submission Attributes Validation (#310) * Pivotal #ID:175163839: Submission Query Operations In Mongo (#319) * Pivotal-176168841: Allow to check inactive user registration (#315) * Pivotal ID # 176522202: Ext Submission serialization (#322) * Pivotal ID # 172671360: Warn users about upcoming public release (#324) * Pivotal #ID 176479707: Mongo implement draft funtionality (#327) * Pivotal #ID 176479720: Mongo Metadata Query (#328) * Pivotal ID # 175163839: Mongo Submission Stats (#331) * Pivotal ID # 176886357: Async Submission Request (#334) * Pivotal ID # 176977556: Delete Submissions In Mongo (#341) * Pivotal ID #176679139: Ext Submission File List Refactor (#344) * Pivotal ID # 177042374: Delete Draft After Submission (#349) * Pivotal ID# 174948695: PTSubmit Delete Operation (#351) * Pivotal #ID 177076728: Use PMC new streaming API (#346) * Pivotal ID # 177267343: Validate EU-ToxRisk Submissions (#352) Bugfix * Pivotal ID # 176052264: Inner Folder Mapping (#311) * Pivotal ID # 172671360: Ignore Empty Value Attributes (#314) * Pivotal ID # 176377632: File list extension is missing in BIA studies (#321) * Pivotal #ID 174642598: TSV Double Quotes (#350) * Pivotal ID # 176501169: FTP Folders Missing (#335) Chore * Pivotal ID # 176377544: Add refresh users directory structure api endpoint (#320) * Pivotal #ID 176479473: Migrate biostudies test to TestContainers (#323) * Pivotal ID # 176779952: Mongo iTests CI (#332) * Pivotal #Id 176838189: Rename PMC mongo collections (#333) * Pivotal #ID: 177040448: Store request as plain json (#336) * Pivotal ID #175894089: Check if queues in RabbitMQ are persistent (#339) * Pivotal ID #177074116 : model table section properly (#338) * Pivotal ID # 176886824: Improve API Docs (#337) * Pivotal #ID 177193125: Improve collection template validation (#345) Co-authored-by: Juan <jcamilorada@ebi.ac.uk>

Pivotal ID # 177267343: Validate EU-ToxRisk Submissions

7b1cd4d

- Implement collection validators - Update the submission submitter to use them

jhoanmanuelms requested a review from Juan-EBI March 16, 2021 15:46

jhoanmanuelms self-assigned this Mar 16, 2021

Juan-EBI reviewed Mar 17, 2021

View reviewed changes

Pivotal ID # 177267343: Validate EU-ToxRisk Submissions

c126c23

- Code review comments - Unit and integration testing

jhoanmanuelms marked this pull request as ready for review March 20, 2021 00:19

Merge branch 'master' into feature/pivotal-177267343-validate-eu-toxrisk

3f525d7

Juan-EBI reviewed Mar 20, 2021

View reviewed changes

Juan-EBI approved these changes Mar 20, 2021

View reviewed changes

Pivotal ID # 177267343: Validate EU-ToxRisk Submissions

d72f141

Code review comments

jhoanmanuelms merged commit d021316 into master Mar 20, 2021

jhoanmanuelms deleted the feature/pivotal-177267343-validate-eu-toxrisk branch March 20, 2021 17:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pivotal ID # 177267343: Validate EU-ToxRisk Submissions #352

Pivotal ID # 177267343: Validate EU-ToxRisk Submissions #352

jhoanmanuelms commented Mar 16, 2021

Juan-EBI Mar 17, 2021 •

edited

Loading

jhoanmanuelms Mar 17, 2021 •

edited

Loading

Juan-EBI Mar 20, 2021

Juan-EBI Mar 20, 2021

jhoanmanuelms Mar 20, 2021

Juan-EBI Mar 20, 2021

Juan-EBI Mar 20, 2021

Juan-EBI Mar 20, 2021

Juan-EBI left a comment


		private fun collectionInfo(accNo: String) = queryService.getBasicCollection(accNo)

		private fun validate(validator: String, submission: ExtSubmission) =

Pivotal ID # 177267343: Validate EU-ToxRisk Submissions #352

Pivotal ID # 177267343: Validate EU-ToxRisk Submissions #352

Conversation

jhoanmanuelms commented Mar 16, 2021

Juan-EBI Mar 17, 2021 • edited Loading

Choose a reason for hiding this comment

jhoanmanuelms Mar 17, 2021 • edited Loading

Choose a reason for hiding this comment

Juan-EBI Mar 20, 2021

Choose a reason for hiding this comment

Juan-EBI Mar 20, 2021

Choose a reason for hiding this comment

jhoanmanuelms Mar 20, 2021

Choose a reason for hiding this comment

Juan-EBI Mar 20, 2021

Choose a reason for hiding this comment

Juan-EBI Mar 20, 2021

Choose a reason for hiding this comment

Juan-EBI Mar 20, 2021

Choose a reason for hiding this comment

Juan-EBI left a comment

Choose a reason for hiding this comment

Juan-EBI Mar 17, 2021 •

edited

Loading

jhoanmanuelms Mar 17, 2021 •

edited

Loading