search

harvard-lil / perma

Indelible links
408 stars 72 forks source link

Rek test link mgmt #3469

Closed kilbergr closed 5 months ago

kilbergr commented 6 months ago

Two things:

  1. I am doing this: https://github.com/harvard-lil/perma/compare/develop...kilbergr:perma:rek-test-link-mgmt?expand=1#diff-7647024a9efc6c8bce2a3b02675bfd7410fb12302f16b7c4bd815391088ca407R508 because I can't pass secure=True into super().generic(*args, **kwargs) because it is already set in kwargs (so I get this error: TypeError: django.test.client.RequestFactory.generic() got multiple values for keyword argument 'secure'). Lmk if this is objectionable.
  2. I added a response.status_code check to the last test, which didn't have one: https://github.com/harvard-lil/perma/compare/develop...kilbergr:perma:rek-test-link-mgmt?expand=1#diff-3d65703d1dab85c7f9193746596fd9d55e3dd667ebe8a10aede02e538ae68ec1R67. However, I can't tell from the test's name whether I should be expecting a different response (i.e. does permitted mean you're allowed to delete this link, and therefore this should be a 200 or is permitted a link type?). Do you know? It passes on a 404, which from this: https://github.com/harvard-lil/perma/blob/f5e43c21a39e61bf7b9dd1b63a925acbfc387678/perma_web/perma/views/link_management.py#L45 suggests this is a not deletable link (request.user.can_delete(link) returns False.
codecov[bot] commented 6 months ago

Codecov Report

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

Comparison is base (f5e43c2) 71.14% compared to head (74b8594) 71.19%. Report is 2 commits behind head on develop.

Files Patch % Lines
perma_web/conftest.py 88.88% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #3469 +/- ## =========================================== + Coverage 71.14% 71.19% +0.04% =========================================== Files 48 48 Lines 6435 6453 +18 =========================================== + Hits 4578 4594 +16 - Misses 1857 1859 +2 ```

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

rebeccacremona commented 5 months ago

sorry for the delay, @kilbergr! i promise to take a look early on Tues!

rebeccacremona commented 5 months ago

(And if you don't want to do these thing.... holler, and I'll send you a commit :-)

kilbergr commented 5 months ago

@rebeccacremona that makes a lot of sense. I've made the suggested changes I believe. Let me know if this wasn't what you were thinking. Thank you!