vrk-kpa / ckanext-matomo

GNU Affero General Public License v3.0
3 stars 3 forks source link

AV-2019: Remove str encoding as unnecessary #36

Closed eetumans closed 1 year ago

eetumans commented 1 year ago

Report generation breaks when trying to print package name to be skipped when package id or name exists in matomo data but no longer in ckan. While the name nor id should include characters that break the encode, the encoding itself should also no longer be necessary in python3 anyways.

github-actions[bot] commented 1 year ago

Pull Request Test Coverage Report for Build 5198495511


Changes Missing Coverage Covered Lines Changed/Added Lines %
ckanext/matomo/commands.py 0 4 0.0%
<!-- Total: 0 4 0.0% -->
Totals Coverage Status
Change from base Build 5198070235: 0.0%
Covered Lines: 582
Relevant Lines: 1331

💛 - Coveralls
eetumans commented 1 year ago

I wasn't quite happy to let it go without finding the actual reason for tripping the encoding. Turns out the regex wasn't quite refined enough to make sure the id_or_name is actually a valid package id or name and thus matched bad requests like package_show?id=”id” where the quotes tripped the encoding. Original quick fix of removing the encoding should have been enough to stop the report generation from breaking and it would've instead just caused another print of Package "”id”" not found, skipping... but I suppose it's still good to verify and disregard invalid data