test: seed data for permission and roles e2e tests #4173
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
novakzaballa
changed the title
|
Uffizzi Preview |
novakzaballa
requested review from
a team and
matthewelwell
and removed request for
a team
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4173 +/- ##
==========================================
+ Coverage 96.50% 96.91% +0.41%
==========================================
Files 1177 1177
Lines 38217 39412 +1195
==========================================
+ Hits 36883 38198 +1315
+ Misses 1334 1214 -120 ☔ View full report in Codecov by Sentry. |
matthewelwell
force-pushed
the
test/seed-data-for-permission-and-roles-e2e-tests
branch
from
3e964dc
to
9e06563
Compare
api/projects/permissions.py
Outdated
| elif ( | ||
| super().has_permission(request, view) | ||
| and getattr(request, "is_e2e", False) is True | ||
| ): | ||
| return True |
There was a problem hiding this comment.
@novakzaballa even if this was necessary, the call to super() is quite clearly unnecessary as we're early returning on the line above if it's not true.
In general though, this is exactly the job that the E2ETestPermissionMixin is trying to achieve. Can you have another look at this please?
There was a problem hiding this comment.
You’re right, the call to super() was no longer necessary and has been corrected.
There was an issue with this part of the code that was validating the number of projects and returning False, I moved part of the logic exclusively to this validation.
There was a problem hiding this comment.
In this case, I don't think the mixin is actually doing anything? I would rather see the mixin used, or removed if we can't get it working.
There was a problem hiding this comment.
I removed it since it doesn't do what we expect.
matthewelwell
dismissed
zachaysan’s stale review
Found issue with subsequent commit.
novakzaballa
deleted the
test/seed-data-for-permission-and-roles-e2e-tests
branch
Thanks for submitting a PR! Please check the boxes below:
pre-committo check lintingdocs/if required so people know about the feature!Changes
How did you test this code?