Skip to content

Remove limit on number of outbound external attachments#130

Merged
adrianclay merged 2 commits intoNHSDigital:developfrom
adrianclay:NIAD-2348/increase-number-of-external-attachments
Sep 18, 2023
Merged

Remove limit on number of outbound external attachments#130
adrianclay merged 2 commits intoNHSDigital:developfrom
adrianclay:NIAD-2348/increase-number-of-external-attachments

Conversation

@adrianclay
Copy link
Copy Markdown
Contributor

@adrianclay adrianclay commented Jul 7, 2023

The original limit appeared erroneous, and not part of the specification.

  • Tests

There exists a test which covers the limit for attachments, but no similar test for external_attachments so I suspect that this validation code was indeed a mistake. Adding confidence to its removal.

@adrianclay
Copy link
Copy Markdown
Contributor Author

@benhession Having a look at the code, I can see no reason for the inbound to perform any validation on the number of external attachments. If you'd like me to add an automated test, I'd be happy to.

self.assertEqual(response.code, 200)
self.assertEqual(response.headers["Correlation-Id"], CORRELATION_ID)

def test_request_with_10_000_doesnt_error(self):
Copy link
Copy Markdown
Contributor Author

@adrianclay adrianclay Sep 15, 2023

Choose a reason for hiding this comment

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

I have a suspicion that these tests could be refactored a bit, but I didn't get a chance to look into this yet.

Happy to pick this up next week however.

@adrianclay adrianclay changed the title Increase limit on number of external attachments Remove limit on number of external attachments Sep 15, 2023
@adrianclay adrianclay changed the title Remove limit on number of external attachments Remove limit on number of outbound external attachments Sep 15, 2023
@adrianclay adrianclay merged commit ac970e4 into NHSDigital:develop Sep 18, 2023
@adrianclay adrianclay deleted the NIAD-2348/increase-number-of-external-attachments branch September 18, 2023 09:16
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.

2 participants