refactor tab modification by components #6549
Conversation
tymofij
force-pushed
the
tim/modify_tabs
branch
from
1ea8833
to
3a3cde0
Compare
| else: | ||
| # If not iterable, return immediately and let validation handle. | ||
| return | ||
| def _a_component_is_present(request, advanced_components): |
There was a problem hiding this comment.
I'd prefer to name it so is_advanced_module_present.
There was a problem hiding this comment.
Looks like _a_component_is_present and _component_value_true get the same arguments, but argument's names differ. I'd like to see the same names in both function. For example, components.
There was a problem hiding this comment.
let's settle on is_advanced_component_present - they are called components all over
tymofij
force-pushed
the
tim/modify_tabs
branch
2 times, most recently
from
734ddeb
to
b157949
Compare
|
|
||
|
|
||
| # pylint: disable=invalid-name | ||
| def _config_course_settings(request, course_module, filter_tabs=True): | ||
| def _component_value_true(request, component_types): |
There was a problem hiding this comment.
I'd prefer to name it so is_field_value_true or is_truthy.
There was a problem hiding this comment.
Do we really need to pass the request as an argument or we can pass request.json instead?
There was a problem hiding this comment.
is_field_value_true it is
|
👍 once tests pass and small comments will be addressed. |
tymofij
force-pushed
the
tim/modify_tabs
branch
from
8a04711
to
56a75b5
Compare
tymofij
force-pushed
the
tim/modify_tabs
branch
from
56a75b5
to
c384ee5
Compare
refactor tab modification by components
@polesye please take a look