[change] Remove customization for checks and replies #336 #360

totallynotvaishnav · 2021-12-24T14:34:02Z

Closes #336

totallynotvaishnav · 2022-01-16T14:00:25Z

@devkapilbansal could you review it?

devkapilbansal

Thanks @totallynotvaishnav
I haven't tested it but looks like the PR is in right direction.

Here, are some changes that I can suggest 👇

P.S. :- Documentation needs to be updated too

devkapilbansal · 2022-01-16T15:04:49Z

openwisp_radius/base/forms.py

@@ -43,7 +43,6 @@ class RadiusCheckForm(ModeSwitcherForm):
        label=_('Value'),
        required=False,
        max_length=radcheck_value_field.max_length,
-        widget=forms.PasswordInput(),


Why this field is converted to CharField

My bad. I thought it was no longer needed.

devkapilbansal · 2022-01-16T15:10:09Z

openwisp_radius/admin.py

        'created',
        'modified',
    ]
    search_fields = ['username', 'value']
    list_filter = [
        DuplicateListFilter,
        ('organization', MultitenantOrgFilter),
-        ExpiredListFilter,


This filter should also be removed from all filters

Will remove it.

devkapilbansal · 2022-01-21T10:14:13Z

openwisp_radius/admin.py

@@ -19,12 +19,7 @@
 )

 from . import settings as app_settings
-from .base.admin_actions import disable_action, enable_action


I can't see these two removed anywhere

I removed it in line 115.

If we don't need disable_action and enable_action, they should be removed too. See here 👇

https://github.com/totallynotvaishnav/openwisp-radius/blob/issues/336-remove-customizations-for-checks-and-replies/openwisp_radius/base/admin_actions.py#L7-L37

devkapilbansal

@totallynotvaishnav I would suggest you to run tests locally too. Also, coverage can be a go to way if you want to know whether the code written is covered by tests or not.

For e.g. :- See that the coverage for admin actions file is decreased to 0

devkapilbansal · 2022-01-29T06:59:08Z

openwisp_radius/admin.py

@@ -19,12 +19,7 @@
 )

 from . import settings as app_settings
-from .base.admin_actions import disable_action, enable_action


If we don't need disable_action and enable_action, they should be removed too. See here 👇

https://github.com/totallynotvaishnav/openwisp-radius/blob/issues/336-remove-customizations-for-checks-and-replies/openwisp_radius/base/admin_actions.py#L7-L37

totallynotvaishnav · 2022-01-29T11:36:12Z

I would suggest you to run tests locally too

@devkapilbansal All tests are successful locally. Don't know why it fails here.

coveralls · 2022-02-04T12:16:10Z

Coverage decreased (-0.01%) to 99.014% when pulling f4bb8ed on totallynotvaishnav:issues/336-remove-customizations-for-checks-and-replies into fd7c2e1 on openwisp:master.

devkapilbansal

@totallynotvaishnav tests pass successfully now. I rebased your branch and made minor improvements. Will check your changes soon

P.S. :- I saw some noisy output in tests here. Can you please open an issue for this ? Would be great if a PR is opened too.

totallynotvaishnav · 2022-02-04T17:14:17Z

Can you please open an issue for this ? Would be great if a PR is opened too.

Sure, will do.

devkapilbansal

@totallynotvaishnav I can find some instances where is_active field is present. It is there in docs too. Please ensure that all necessary changes are done

totallynotvaishnav · 2022-03-10T10:33:51Z

I can find some instances where is_active field is present. It is there in docs too

@devkapilbansal I don't see any is_active field related to checks. Could you link to any reference?

closes openwisp#336

nemesifier

It looks good, thank you @totallynotvaishnav @devkapilbansal 👍!
I have tested it a bit and will test more in the next days.

nemesifier · 2022-04-12T13:48:33Z

There were quite a few things remaining to be deleted, I took are of those in 56cf9f1.

totallynotvaishnav force-pushed the issues/336-remove-customizations-for-checks-and-replies branch from 7b32cac to 87a37ef Compare December 24, 2021 14:42

devkapilbansal requested changes Jan 16, 2022

View reviewed changes

pandafy added this to To do (general) in OpenWISP Contributor's Board via automation Jan 19, 2022

pandafy moved this from To do (general) to In progress in OpenWISP Contributor's Board Jan 19, 2022

devkapilbansal reviewed Jan 21, 2022

View reviewed changes

totallynotvaishnav requested a review from devkapilbansal January 28, 2022 10:23

devkapilbansal requested changes Jan 29, 2022

View reviewed changes

totallynotvaishnav closed this Jan 29, 2022

OpenWISP Contributor's Board automation moved this from In progress to Done Jan 29, 2022

totallynotvaishnav reopened this Jan 29, 2022

OpenWISP Contributor's Board automation moved this from Done to In progress Jan 29, 2022

totallynotvaishnav force-pushed the issues/336-remove-customizations-for-checks-and-replies branch 2 times, most recently from fb93c52 to 88fd655 Compare January 30, 2022 11:23

totallynotvaishnav requested a review from devkapilbansal February 4, 2022 09:40

devkapilbansal force-pushed the issues/336-remove-customizations-for-checks-and-replies branch from 88fd655 to 1a3b352 Compare February 4, 2022 11:52

devkapilbansal moved this from In progress to Needs review in OpenWISP Contributor's Board Feb 4, 2022

devkapilbansal reviewed Feb 4, 2022

View reviewed changes

devkapilbansal mentioned this pull request Feb 15, 2022

[tests] Avoid error logging to print noisy output during tests #377

Closed

pandafy added this to In progress in OpenWISP Priorities for next releases via automation Feb 18, 2022

pandafy moved this from In progress to Ready for review/testing in OpenWISP Priorities for next releases Feb 18, 2022

pandafy requested review from pandafy, nemesifier and devkapilbansal February 18, 2022 14:57

devkapilbansal requested changes Feb 18, 2022

View reviewed changes

OpenWISP Contributor's Board automation moved this from Needs review to In progress Feb 18, 2022

OpenWISP Priorities for next releases automation moved this from Ready for review/testing to In progress Feb 18, 2022

totallynotvaishnav requested a review from devkapilbansal March 10, 2022 10:48

totallynotvaishnav and others added 7 commits April 8, 2022 18:18

[change] Remove customizations for checks and replies openwisp#336

c30be85

closes openwisp#336

[change] Updated migrations

c601cf2

[change] updated tests

9343fff

[change] Updated docs

e7c1ce2

[change] Remove Enable and Disable admin actions

b25eee3

[change] Cleanup filters that are not longer needed

23ebe87

[chores] Rename migration name

f454fd7

nemesifier force-pushed the issues/336-remove-customizations-for-checks-and-replies branch from 9f4f03a to 6296a03 Compare April 8, 2022 22:19

nemesifier reviewed Apr 8, 2022

View reviewed changes

nemesifier force-pushed the issues/336-remove-customizations-for-checks-and-replies branch from 6296a03 to 27f12df Compare April 8, 2022 22:22

[fix] Fixed migration

f4bb8ed

nemesifier force-pushed the issues/336-remove-customizations-for-checks-and-replies branch from 27f12df to f4bb8ed Compare April 8, 2022 22:32

nemesifier merged commit f4bb8ed into openwisp:master Apr 11, 2022

OpenWISP Contributor's Board automation moved this from In progress to Done Apr 11, 2022

OpenWISP Priorities for next releases automation moved this from In progress to Done Apr 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[change] Remove customization for checks and replies #336 #360

[change] Remove customization for checks and replies #336 #360

totallynotvaishnav commented Dec 24, 2021

totallynotvaishnav commented Jan 16, 2022

devkapilbansal left a comment

devkapilbansal Jan 16, 2022

totallynotvaishnav Jan 17, 2022

devkapilbansal Jan 16, 2022

totallynotvaishnav Jan 17, 2022

devkapilbansal Jan 21, 2022

totallynotvaishnav Jan 21, 2022

devkapilbansal Jan 29, 2022

devkapilbansal left a comment

devkapilbansal Jan 29, 2022

totallynotvaishnav commented Jan 29, 2022 •

edited

Loading

coveralls commented Feb 4, 2022 •

edited

Loading

devkapilbansal left a comment

totallynotvaishnav commented Feb 4, 2022

devkapilbansal left a comment

totallynotvaishnav commented Mar 10, 2022

nemesifier left a comment

nemesifier commented Apr 12, 2022

[change] Remove customization for checks and replies #336 #360

[change] Remove customization for checks and replies #336 #360

Conversation

totallynotvaishnav commented Dec 24, 2021

totallynotvaishnav commented Jan 16, 2022

devkapilbansal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devkapilbansal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

totallynotvaishnav commented Jan 29, 2022 • edited Loading

coveralls commented Feb 4, 2022 • edited Loading

devkapilbansal left a comment

Choose a reason for hiding this comment

totallynotvaishnav commented Feb 4, 2022

devkapilbansal left a comment

Choose a reason for hiding this comment

totallynotvaishnav commented Mar 10, 2022

nemesifier left a comment

Choose a reason for hiding this comment

nemesifier commented Apr 12, 2022

totallynotvaishnav commented Jan 29, 2022 •

edited

Loading

coveralls commented Feb 4, 2022 •

edited

Loading