-
Notifications
You must be signed in to change notification settings - Fork 6
FFI-8 P4: fix pylint failures #608
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1451,7 +1451,7 @@ def update_enrollment(self, mode=None, is_active=None, skip_refund=False): | |
| pii=UserPersonalData( | ||
| username=self.user.username, | ||
| email=self.user.email, | ||
| name=self.user.profile.name, | ||
| name=self.user.profile.name, # pylint: disable=no-member | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't disable this violation in this PR: #567 |
||
| ), | ||
| id=self.user.id, | ||
| is_active=self.user.is_active, | ||
|
|
@@ -1477,7 +1477,7 @@ def update_enrollment(self, mode=None, is_active=None, skip_refund=False): | |
| pii=UserPersonalData( | ||
| username=self.user.username, | ||
| email=self.user.email, | ||
| name=self.user.profile.name, | ||
| name=self.user.profile.name, # pylint: disable=no-member | ||
| ), | ||
| id=self.user.id, | ||
| is_active=self.user.is_active, | ||
|
|
@@ -1683,7 +1683,7 @@ def enroll(cls, user, course_key, mode=None, check_access=False, can_upgrade=Fal | |
| pii=UserPersonalData( | ||
| username=user.username, | ||
| email=user.email, | ||
| name=user.profile.name, | ||
| name=user.profile.name, # pylint: disable=no-member | ||
| ), | ||
| id=user.id, | ||
| is_active=user.is_active, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,10 +10,6 @@ | |
| from django.test import TestCase | ||
| from django_countries.fields import Country | ||
|
|
||
| from common.djangoapps.student.models import CourseEnrollmentAllowed, CourseEnrollment | ||
| from common.djangoapps.student.tests.factories import CourseEnrollmentAllowedFactory, UserFactory, UserProfileFactory | ||
| from common.djangoapps.student.tests.tests import UserSettingsEventTestMixin | ||
|
|
||
| from openedx_events.learning.data import ( | ||
| CourseData, | ||
| CourseEnrollmentData, | ||
|
|
@@ -26,8 +22,11 @@ | |
| COURSE_UNENROLLMENT_COMPLETED, | ||
| ) | ||
| from openedx_events.tests.utils import OpenEdxEventsTestMixin | ||
| from openedx.core.djangolib.testing.utils import skip_unless_lms | ||
|
|
||
| from common.djangoapps.student.models import CourseEnrollmentAllowed, CourseEnrollment | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't fix those imports in this PR: #567 |
||
| from common.djangoapps.student.tests.factories import CourseEnrollmentAllowedFactory, UserFactory, UserProfileFactory | ||
| from common.djangoapps.student.tests.tests import UserSettingsEventTestMixin | ||
| from openedx.core.djangolib.testing.utils import skip_unless_lms | ||
| from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase | ||
| from xmodule.modulestore.tests.factories import CourseFactory | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,25 @@ | ||
| """ | ||
| Test that various filters are fired for models in the student app. | ||
| """ | ||
| from django.test import override_settings | ||
| from openedx_filters.learning.enrollment import PreEnrollmentFilter | ||
| from openedx_filters import PipelineStep | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't fix these imports in this PR: #573 |
||
| from common.djangoapps.student.models import CourseEnrollment, EnrollmentNotAllowed | ||
| from common.djangoapps.student.tests.factories import UserFactory, UserProfileFactory | ||
|
|
||
| from openedx.core.djangolib.testing.utils import skip_unless_lms | ||
| from django.test import TestCase, override_settings | ||
| from openedx_filters.learning.enrollment import PreEnrollmentFilter | ||
|
|
||
|
|
||
| from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase | ||
| from xmodule.modulestore.tests.factories import CourseFactory | ||
|
|
||
| from openedx_filters import PipelineStep | ||
|
|
||
|
|
||
| class TestEnrollmentPipelineStep(PipelineStep): | ||
| """ | ||
| Utility function used when getting steps for pipeline. | ||
| """ | ||
|
|
||
| def run(self, user, course_key, mode): | ||
| def run(self, user, course_key, mode): # pylint: disable=unused-argument, arguments-differ | ||
| """Pipeline steps that changes mode to honor.""" | ||
| if mode == "no-id-professional": | ||
| raise PreEnrollmentFilter.PreventEnrollment() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,8 @@ | |
| from model_utils.models import TimeStampedModel | ||
| from opaque_keys.edx.django.models import CourseKeyField | ||
| from simple_history.models import HistoricalRecords | ||
| from openedx_events.learning.data import CourseData, UserData, UserPersonalData, CertificateData | ||
| from openedx_events.learning.signals import CERTIFICATE_CHANGED, CERTIFICATE_CREATED, CERTIFICATE_REVOKED | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't fix those imports in this PR: #567 |
||
|
|
||
| from common.djangoapps.course_modes.models import CourseMode | ||
| from common.djangoapps.util.milestones_helpers import fulfill_course_milestone, is_prerequisite_courses_enabled | ||
|
|
@@ -33,8 +35,6 @@ | |
| from openedx.core.djangoapps.signals.signals import COURSE_CERT_AWARDED, COURSE_CERT_CHANGED, COURSE_CERT_REVOKED | ||
| from openedx.core.djangoapps.xmodule_django.models import NoneToEmptyManager | ||
|
|
||
| from openedx_events.learning.data import CourseData, UserData, UserPersonalData, CertificateData | ||
| from openedx_events.learning.signals import CERTIFICATE_CHANGED, CERTIFICATE_CREATED, CERTIFICATE_REVOKED | ||
|
|
||
| log = logging.getLogger(__name__) | ||
| User = get_user_model() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,6 @@ | |
|
|
||
| from unittest import mock | ||
|
|
||
| from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't remove this unused import in this PR: #566 |
||
| from django.test.utils import override_settings | ||
| from django.urls import reverse | ||
| from rest_framework.test import APIClient | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,11 +13,11 @@ | |
| from django.dispatch import receiver | ||
| from django.utils.encoding import python_2_unicode_compatible | ||
| from opaque_keys.edx.django.models import CourseKeyField | ||
| from openedx_events.learning.data import CohortData, CourseData, UserData, UserPersonalData | ||
| from openedx_events.learning.signals import COHORT_MEMBERSHIP_CHANGED | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't fix those imports in this PR: #567 |
||
|
|
||
| from openedx.core.djangolib.model_mixins import DeletableByUserValue | ||
|
|
||
| from openedx_events.learning.data import CohortData, CourseData, UserData, UserPersonalData | ||
| from openedx_events.learning.signals import COHORT_MEMBERSHIP_CHANGED | ||
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
|
|
@@ -139,7 +139,7 @@ def save(self, force_insert=False, force_update=False, using=None, update_fields | |
| pii=UserPersonalData( | ||
| username=self.user.username, | ||
| email=self.user.email, | ||
| name=self.user.profile.name, | ||
| name=self.user.profile.name, # pylint: disable=no-member | ||
| ), | ||
| id=self.user.id, | ||
| is_active=self.user.is_active, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,19 +4,16 @@ | |
| Classes: | ||
| CohortEventTest: Test event sent after cohort membership changes. | ||
| """ | ||
| from openedx.core.djangoapps.course_groups.models import CohortMembership | ||
| from unittest.mock import Mock | ||
|
|
||
| from openedx_events.learning.data import CohortData, CourseData, UserData, UserPersonalData | ||
| from openedx_events.learning.signals import COHORT_MEMBERSHIP_CHANGED | ||
| from openedx_events.tests.utils import OpenEdxEventsTestMixin | ||
|
|
||
| from openedx.core.djangoapps.course_groups.models import CohortMembership | ||
| from common.djangoapps.student.tests.factories import UserFactory | ||
| from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory | ||
| from openedx.core.djangolib.testing.utils import skip_unless_lms | ||
|
|
||
| from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't fix those imports in this PR: #567 |
||
| from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,7 +87,7 @@ def save_bearer_token(self, token, request, *args, **kwargs): | |
| # Without this modification the BearerToken class will set this to 3600 | ||
| request.expires_in = getattr(settings, 'CLIENT_CREDENTIALS_ACCESS_TOKEN_EXPIRE_SECONDS', 31557600) | ||
|
|
||
| super(EdxOAuth2Validator, self).save_bearer_token(token, request, *args, **kwargs) | ||
| super().save_bearer_token(token, request, *args, **kwargs) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't modernize this super here: #595 |
||
|
|
||
| is_restricted_client = self._update_token_expiry_if_restricted_client(token, request.client) | ||
| if not is_restricted_client: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -253,7 +253,7 @@ def create_account_with_params(request, params): | |
| pii=UserPersonalData( | ||
| username=user.username, | ||
| email=user.email, | ||
| name=user.profile.name, | ||
| name=user.profile.name, # pylint: disable=no-member | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't disable this violation here: #567 |
||
| ), | ||
| id=user.id, | ||
| is_active=user.is_active, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,8 @@ | |
| from django.urls import NoReverseMatch, reverse | ||
| from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch | ||
| from freezegun import freeze_time | ||
| from common.djangoapps.student.tests.factories import RegistrationFactory, UserFactory, UserProfileFactory | ||
| from openedx_events.tests.utils import OpenEdxEventsTestMixin | ||
| from common.djangoapps.student.tests.factories import RegistrationFactory, UserFactory, UserProfileFactory | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We didn't fix those imports in this PR: #567 |
||
|
|
||
| from openedx.core.djangoapps.password_policy.compliance import ( | ||
| NonCompliantPasswordException, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We didn't need this method after this PR: #574