harvard-lil / perma

Indelible links
413 stars 70 forks source link

Check `targetUrlResolved` against `PRIVATE_BY_POLICY_DOMAINS` #3405

Closed rebeccacremona closed 11 months ago

rebeccacremona commented 11 months ago

In https://github.com/harvard-lil/perma/pull/3392, we implemented a new policy for when to make Perma Links private: we maintain a list of domains to check against.

At the time, when capturing with Scoop, it was only possible for Perma to check the target URL.

As of Scoop 0.5.5, added to the Scoop API recently, we can now also see the URL that the browser "lands on", after any redirects, and check that too.

This PR does so.

See ENG-352.


Note: I spent a lot of time yesterday trying to add a test for this, experimenting with 3 approaches, none of which worked in all circumstances, and all of which were intricate. For a feature of this importance.... I think it is more appropriate to leave it untested. I'm happy to say more, if people are curious.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (42fbdd7) 68.57% compared to head (fa938e9) 68.65%. Report is 5 commits behind head on develop.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #3405 +/- ## =========================================== + Coverage 68.57% 68.65% +0.08% =========================================== Files 53 53 Lines 7229 7233 +4 =========================================== + Hits 4957 4966 +9 + Misses 2272 2267 -5 ``` | [Files](https://app.codecov.io/gh/harvard-lil/perma/pull/3405?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=harvard-lil) | Coverage Δ | | |---|---|---| | [perma\_web/api/urls.py](https://app.codecov.io/gh/harvard-lil/perma/pull/3405?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=harvard-lil#diff-cGVybWFfd2ViL2FwaS91cmxzLnB5) | `80.95% <100.00%> (+0.95%)` | :arrow_up: | | [perma\_web/perma/celery\_tasks.py](https://app.codecov.io/gh/harvard-lil/perma/pull/3405?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=harvard-lil#diff-cGVybWFfd2ViL3Blcm1hL2NlbGVyeV90YXNrcy5weQ==) | `47.82% <0.00%> (+0.46%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/harvard-lil/perma/pull/3405/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=harvard-lil)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.