Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: hookup feature models to django admin #413

Merged
merged 10 commits into from
Feb 28, 2024

Conversation

daniel-codecov
Copy link
Contributor

@daniel-codecov daniel-codecov commented Feb 23, 2024

Purpose/Motivation

Screen.Recording.2024-02-26.at.2.30.56.PM.mov

Adds the new feature models from here to django admin so that we can assign feature variants to features dynamically

Copy link
Contributor

@matt-codecov matt-codecov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you upload a screenshot of what the webform looks like or something? very curious if it is actually usable OOTB or if we have to do some custom webform work to allow users to input things like "github/codecov" and have that translate to the database id for codecov's github org

@@ -16,7 +16,6 @@
"django_http_requests_unknown_latency_including_middlewares_total",
"django_http_requests_latency_seconds_by_view_method",
"django_http_requests_unknown_latency_total",
"django_http_ajax_requests_total",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this metric isn't actually important for us (and makes django-autocomplete-light error)

views.RepositoryAutoCompleteSearch.as_view(),
name="admin-repository-autocomplete",
),
path(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these endpoints will be exposed on the API but I think its fine cause of this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for calling this out!

codecov/views.py Outdated
class RepositoryAutoCompleteSearch(autocomplete.Select2QuerySetView):
def get_queryset(self):
# must be authenticated to query
if not self.request.user.is_authenticated:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think if you do self.request.user.is_staff instead of is_authenticated it's just users who can access the admin interface

views.RepositoryAutoCompleteSearch.as_view(),
name="admin-repository-autocomplete",
),
path(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for calling this out!

codecov/views.py Outdated
class OwnerAutoCompleteSearch(autocomplete.Select2QuerySetView):
def get_queryset(self):
# must be authenticated to query
if not self.request.user.is_authenticated:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same is_staff comment as above

@codecov-qa
Copy link

codecov-qa bot commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 62.50000% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 95.94%. Comparing base (35fa8e4) to head (74dab69).

Files Patch % Lines
codecov/forms.py 50.00% 12 Missing ⚠️
codecov/views.py 36.84% 12 Missing ⚠️
codecov/admin.py 89.65% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #413      +/-   ##
==========================================
- Coverage   96.09%   95.94%   -0.15%     
==========================================
  Files         640      641       +1     
  Lines       16867    16938      +71     
==========================================
+ Hits        16208    16252      +44     
- Misses        659      686      +27     
Flag Coverage Δ
unit 95.94% <62.50%> (-0.15%) ⬇️
unit-latest-uploader 95.94% <62.50%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

codecov-public-qa bot commented Feb 28, 2024

Codecov Report

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

Comparison is base (35fa8e4) 96.09% compared to head (74dab69) 95.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #413      +/-   ##
==========================================
- Coverage   96.09%   95.94%   -0.15%     
==========================================
  Files         640      641       +1     
  Lines       16867    16938      +71     
==========================================
+ Hits        16208    16252      +44     
- Misses        659      686      +27     
Flag Coverage Δ
unit 95.94% <62.50%> (-0.15%) ⬇️
unit-latest-uploader 95.94% <62.50%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
codecov/settings_base.py 85.86% <ø> (ø)
codecov/urls.py 100.00% <ø> (ø)
core/middleware.py 100.00% <ø> (ø)
codecov/admin.py 92.85% <89.65%> (-7.15%) ⬇️
codecov/forms.py 50.00% <50.00%> (ø)
codecov/views.py 64.70% <36.84%> (-35.30%) ⬇️

Impacted file tree graph

@codecov-staging
Copy link

Codecov Report

Attention: Patch coverage is 62.50000% with 27 lines in your changes are missing coverage. Please review.

Files Patch % Lines
codecov/forms.py 50.00% 12 Missing ⚠️
codecov/views.py 36.84% 12 Missing ⚠️
codecov/admin.py 89.65% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@daniel-codecov daniel-codecov merged commit 4fe7a73 into main Feb 28, 2024
13 of 18 checks passed
@daniel-codecov daniel-codecov deleted the daniel/django-admin-for-rollouts branch February 28, 2024 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants