@sgiehl opened this Pull Request on December 9th 2020 Member

Description:

When sending a request like this:

{"module":"API","format":"rss","pageUrl":"\xxxxxxxxxxxx","idSite":"1","period":"day","date":"last7","token_auth":"XYZANONYMIZED","method":"Actions.getPageUrl","filter_limit":100}

the api method Actions.getPageUrl currently might return a completely empty datatable, if the url can't be found. But the RSS renderer tries to use the period metadata of the datatable, which does not exist, and fails.

Simply returning the first subtables metadata for the empty table response solves that issue.

fixes #16918

Review

  • [ ] Functional review done
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@diosmosis commented on December 11th 2020 Member

It's a bit clearer, but I'm still unsure as to whether the period we pick in the list of tables will be what is expected by callers. I also noticed that this code doesn't appear to be tested (there don't seem to be any tests that call getPageUrl/getOutlink/etc.). Do you think it would be a good idea to add an integration or system test to cover this behavior and maybe check the period that is returned? If it's there isn't one already, perhaps there could be a rss test for this as well?

@sgiehl commented on January 14th 2021 Member

@diosmosis I've just added some RSS tests

@diosmosis commented on January 17th 2021 Member

@sgiehl is there a test that requests a page URL that cannot be found (to trigger the bug this PR fixes)?

@sgiehl commented on January 18th 2021 Member

@diosmosis I've just added a test that should throw an exception on 4.x-dev

This Pull Request was closed on January 19th 2021
Powered by GitHub Issue Mirror