Skip to content

fix: Enable editing of related liaison statement information#6371

Merged
rjsparks merged 1 commit intoietf-tools:mainfrom
larseggert:fix-6264
Sep 25, 2023
Merged

fix: Enable editing of related liaison statement information#6371
rjsparks merged 1 commit intoietf-tools:mainfrom
larseggert:fix-6264

Conversation

@larseggert
Copy link
Copy Markdown
Collaborator

Fixes #6264

Comment thread ietf/liaisons/tests.py
try:
decoded = json.loads(json_data)
except json.JSONDecodeError as e:
self.fail('data-pre contained invalid JSON data: %s' % str(e))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a little odd. The test is "Does the code that builds the form put JSON in the field". I guess it's better to break of there's something that's not clearly JSON there, but a lot could go in that would survive json.loads that isn't useful.

self.fail outside of something like setUp where you are using it to check that the test framework and factory/fixture data is sane is always a bit of a red flag.

A better test would look to see that the fieid contained the expected data?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We'll move forward with this and refine it later if needed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That pattern exists elsewhere in the tests, so we should fix it everywhere.

@rjsparks rjsparks merged commit 7300859 into ietf-tools:main Sep 25, 2023
@larseggert larseggert deleted the fix-6264 branch September 26, 2023 06:01
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The Related liaison field of the edit liaison form is broken

2 participants