Modify the revision raw message display approach
Details
Details
- Reviewers
ardumont - Group Reviewers
Reviewers - Commits
- R65:3fc92ddfb078: utils: add logic to calculate raw revision message url to enrich_revision…
R65:bd480fa6df2a: revision, revision-log: update jinja2 to reflect changes to raw message url…
R65:176280f7a10c: utils, test_utils: refactor to eliminate useless tests and avoid redundant…
R65:e9360a46f830: test_utils: rename helper function to show it's a helper
R65:70d27ee04733: converters, service: * remove url logic to fit with converters being api…
R65:fe8b01656b71: templates: update revision and revision-log to reflect raw message display…
R65:8635f93bf14c: service: * remove decoding logic from revision message handling, now in…
R65:6c75b9f6c83b: api: update api_revision_raw_message to automatically download the raw revision…
R65:4a59ce752ade: converters: setup revision message decoding here. Update tests to match
R65:9ec1cbb0cc96: browse: add raw rev message browsing route, update tests to match
R65:28276ee94c4b: Converters, Service, Api, Browse: change 'msg_url' to 'message_url'
rDWAPPS3fc92ddfb078: utils: add logic to calculate raw revision message url to enrich_revision…
rDWAPPSe9360a46f830: test_utils: rename helper function to show it's a helper
rDWAPPS70d27ee04733: converters, service: * remove url logic to fit with converters being api…
rDWAPPSfe8b01656b71: templates: update revision and revision-log to reflect raw message display…
rDWAPPS176280f7a10c: utils, test_utils: refactor to eliminate useless tests and avoid redundant…
rDWAPPSbd480fa6df2a: revision, revision-log: update jinja2 to reflect changes to raw message url…
rDWAPPS28276ee94c4b: Converters, Service, Api, Browse: change 'msg_url' to 'message_url'
rDWAPPS9ec1cbb0cc96: browse: add raw rev message browsing route, update tests to match
rDWAPPS6c75b9f6c83b: api: update api_revision_raw_message to automatically download the raw…
rDWAPPS8635f93bf14c: service: * remove decoding logic from revision message handling, now in…
rDWAPPS4a59ce752ade: converters: setup revision message decoding here. Update tests to match
Tests included
Diff Detail
Diff Detail
- Repository
- rDWAPPS Web applications
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
I think the implementation of this is reasonable. I would just prefer that you use message_url instead of msg_url to keep the object keys consistent with the actual field.
| swh/web/ui/converters.py | ||
|---|---|---|
| 182–183 | Please use message_url instead of msg_url for consistency. | |
| swh/web/ui/converters.py | ||
|---|---|---|
| 183 | The converter and the service layer modules are api/view agnostic. It should be the concern of the api layer to test for the message. | |
| swh/web/ui/service.py | ||
| 218 ↗ | (On Diff #138) | That would be here that I'd add the unicode conversion with the trap. |
| swh/web/ui/views/api.py | ||
| 492 ↗ | (On Diff #138) | That'd be here that i'd add the message_url if message is None. |
| swh/web/ui/tests/test_utils.py | ||
|---|---|---|
| 579 ↗ | (On Diff #139) | Can you please remove the print statement? |
| 636 ↗ | (On Diff #139) | If it's the same function as defined in prior test, define it globally once and reuse it. |
| swh/web/ui/utils.py | ||
| 247 ↗ | (On Diff #139) | Instead of the 3 tests before, can't we simplify to one test on 'message_decoding_failed' existence? I mean, isn't this sufficient? if 'message_decoding_failed' in revision:
...? |