Skip to content

Commit 2c2ee32

Browse files
Copilotkizniche
Copilot
and
kizniche
committed
Remove duplicate controller_id from ControllerMod; use form_del.controller_id in update handler
Co-authored-by: kizniche <838427+kizniche@users.noreply.github.com>
1 parent 01800f1 commit 2c2ee32

File tree

5 files changed

+35
-31
lines changed

5 files changed

+35
-31
lines changed

‎mycodo/mycodo_flask/forms/forms_settings.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ class ControllerDel(FlaskForm):
183183

184184

185185
class ControllerMod(FlaskForm):
186-
controller_id = StringField(widget=widgets.HiddenInput())
187186
update_controller_file = FileField()
188187
update_controller = SubmitField(lazy_gettext('Replace'))
189188

‎mycodo/mycodo_flask/routes_settings.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ def settings_function():
136136
elif form_controller_delete.delete_controller.data:
137137
utils_settings.settings_function_delete(form_controller_delete)
138138
elif form_controller_update.update_controller.data:
139-
utils_settings.settings_function_update(form_controller_update)
139+
utils_settings.settings_function_update(form_controller_delete, form_controller_update)
140140

141141
return redirect(url_for('routes_settings.settings_function'))
142142

‎mycodo/mycodo_flask/templates/settings/function.html‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ <h3>{{_('Imported Function Modules')}}</h3>
4545
<form method="post" action="/settings/function" enctype="multipart/form-data">
4646
{{form_controller_delete.csrf_token}}
4747
{{form_controller_delete.controller_id(value=each_controller)}}
48-
{{form_controller_update.controller_id(value=each_controller)}}
4948

5049
<tr>
5150
<td>{{each_controller}}</td>

‎mycodo/mycodo_flask/utils/utils_settings.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ def settings_function_delete(form):
617617
flash_success_errors(error, action, url_for('routes_settings.settings_function'))
618618

619619

620-
def settings_function_update(form):
620+
def settings_function_update(form_del, form):
621621
"""
622622
Receive a function module file, check it for errors, replace the existing module
623623
"""
@@ -637,7 +637,7 @@ def settings_function_update(form):
637637
tmp_name = 'tmp_function_testing.py'
638638
full_path_tmp = os.path.join(tmp_directory, tmp_name)
639639

640-
controller_device_name = form.controller_id.data
640+
controller_device_name = form_del.controller_id.data
641641

642642
if not form.update_controller_file.data:
643643
error.append('No file present')

‎mycodo/tests/software_tests/test_mycodo_flask/test_utils_settings.py‎

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,16 @@ def save(self, dst):
3131
f.write(self._content)
3232

3333

34-
def make_update_form(controller_id, file_storage):
35-
"""Create a mock ControllerMod form object for testing."""
34+
def make_del_form(controller_id):
35+
"""Create a mock ControllerDel form (supplies controller_id)."""
3636
form = MagicMock()
3737
form.controller_id.data = controller_id
38+
return form
39+
40+
41+
def make_mod_form(file_storage):
42+
"""Create a mock ControllerMod form (supplies the replacement file)."""
43+
form = MagicMock()
3844
form.update_controller_file.data = file_storage
3945
return form
4046

@@ -100,14 +106,14 @@ def test_update_valid_module_no_activated_functions(
100106
# Write the existing module that will be read from disk for comparison
101107
write_existing_module(custom_functions_dir)
102108

103-
file_storage = MockFileStorage('my_custom_function.py', VALID_FUNCTION_CONTENT)
104-
form = make_update_form(VALID_FUNCTION_UNIQUE_NAME, file_storage)
109+
form_del = make_del_form(VALID_FUNCTION_UNIQUE_NAME)
110+
form = make_mod_form(MockFileStorage('my_custom_function.py', VALID_FUNCTION_CONTENT))
105111

106112
with patch('mycodo.mycodo_flask.utils.utils_settings.INSTALL_DIRECTORY', tmp_install_dir), \
107113
patch('mycodo.mycodo_flask.utils.utils_settings.PATH_FUNCTIONS_CUSTOM', custom_functions_dir), \
108114
patch('mycodo.mycodo_flask.utils.utils_settings.CustomController') as mock_cc:
109115
mock_cc.query.filter.return_value.count.return_value = 0
110-
settings_function_update(form)
116+
settings_function_update(form_del, form)
111117

112118
# File should be written to the custom functions directory
113119
expected_file = os.path.join(custom_functions_dir, 'my_custom_function.py')
@@ -128,15 +134,15 @@ def test_update_valid_module_with_activated_function(
128134

129135
write_existing_module(custom_functions_dir)
130136

131-
file_storage = MockFileStorage('my_custom_function.py', VALID_FUNCTION_CONTENT)
132-
form = make_update_form(VALID_FUNCTION_UNIQUE_NAME, file_storage)
137+
form_del = make_del_form(VALID_FUNCTION_UNIQUE_NAME)
138+
form = make_mod_form(MockFileStorage('my_custom_function.py', VALID_FUNCTION_CONTENT))
133139

134140
with patch('mycodo.mycodo_flask.utils.utils_settings.INSTALL_DIRECTORY', tmp_install_dir), \
135141
patch('mycodo.mycodo_flask.utils.utils_settings.PATH_FUNCTIONS_CUSTOM', custom_functions_dir), \
136142
patch('mycodo.mycodo_flask.utils.utils_settings.CustomController') as mock_cc:
137143
# One activated CustomController entry exists for this module
138144
mock_cc.query.filter.return_value.count.return_value = 1
139-
settings_function_update(form)
145+
settings_function_update(form_del, form)
140146

141147
# File should exist
142148
assert os.path.exists(os.path.join(custom_functions_dir, 'my_custom_function.py'))
@@ -158,14 +164,14 @@ def test_update_overwrites_existing_module(
158164
write_existing_module(custom_functions_dir, VALID_FUNCTION_CONTENT + b'# old version\n')
159165

160166
new_content = VALID_FUNCTION_CONTENT + b'# new version\n'
161-
file_storage = MockFileStorage('my_custom_function.py', new_content)
162-
form = make_update_form(VALID_FUNCTION_UNIQUE_NAME, file_storage)
167+
form_del = make_del_form(VALID_FUNCTION_UNIQUE_NAME)
168+
form = make_mod_form(MockFileStorage('my_custom_function.py', new_content))
163169

164170
with patch('mycodo.mycodo_flask.utils.utils_settings.INSTALL_DIRECTORY', tmp_install_dir), \
165171
patch('mycodo.mycodo_flask.utils.utils_settings.PATH_FUNCTIONS_CUSTOM', custom_functions_dir), \
166172
patch('mycodo.mycodo_flask.utils.utils_settings.CustomController') as mock_cc:
167173
mock_cc.query.filter.return_value.count.return_value = 0
168-
settings_function_update(form)
174+
settings_function_update(form_del, form)
169175

170176
with open(os.path.join(custom_functions_dir, 'my_custom_function.py'), 'rb') as f:
171177
content = f.read()
@@ -180,11 +186,12 @@ def test_update_no_file_fails(
180186
from mycodo.mycodo_flask.utils.utils_settings import settings_function_update
181187

182188
write_existing_module(custom_functions_dir)
183-
form = make_update_form(VALID_FUNCTION_UNIQUE_NAME, None)
189+
form_del = make_del_form(VALID_FUNCTION_UNIQUE_NAME)
190+
form = make_mod_form(None)
184191

185192
with patch('mycodo.mycodo_flask.utils.utils_settings.INSTALL_DIRECTORY', tmp_install_dir), \
186193
patch('mycodo.mycodo_flask.utils.utils_settings.PATH_FUNCTIONS_CUSTOM', custom_functions_dir):
187-
settings_function_update(form)
194+
settings_function_update(form_del, form)
188195

189196
mock_popen.assert_not_called()
190197

@@ -196,12 +203,12 @@ def test_update_empty_filename_fails(
196203
from mycodo.mycodo_flask.utils.utils_settings import settings_function_update
197204

198205
write_existing_module(custom_functions_dir)
199-
file_storage = MockFileStorage('', VALID_FUNCTION_CONTENT)
200-
form = make_update_form(VALID_FUNCTION_UNIQUE_NAME, file_storage)
206+
form_del = make_del_form(VALID_FUNCTION_UNIQUE_NAME)
207+
form = make_mod_form(MockFileStorage('', VALID_FUNCTION_CONTENT))
201208

202209
with patch('mycodo.mycodo_flask.utils.utils_settings.INSTALL_DIRECTORY', tmp_install_dir), \
203210
patch('mycodo.mycodo_flask.utils.utils_settings.PATH_FUNCTIONS_CUSTOM', custom_functions_dir):
204-
settings_function_update(form)
211+
settings_function_update(form_del, form)
205212

206213
mock_popen.assert_not_called()
207214

@@ -217,14 +224,14 @@ def test_update_different_filename_same_unique_name_succeeds(
217224
write_existing_module(custom_functions_dir)
218225

219226
# Uploaded file has a DIFFERENT filename but the SAME function_name_unique in its dict
220-
file_storage = MockFileStorage('renamed_upload.py', VALID_FUNCTION_CONTENT)
221-
form = make_update_form(VALID_FUNCTION_UNIQUE_NAME, file_storage)
227+
form_del = make_del_form(VALID_FUNCTION_UNIQUE_NAME)
228+
form = make_mod_form(MockFileStorage('renamed_upload.py', VALID_FUNCTION_CONTENT))
222229

223230
with patch('mycodo.mycodo_flask.utils.utils_settings.INSTALL_DIRECTORY', tmp_install_dir), \
224231
patch('mycodo.mycodo_flask.utils.utils_settings.PATH_FUNCTIONS_CUSTOM', custom_functions_dir), \
225232
patch('mycodo.mycodo_flask.utils.utils_settings.CustomController') as mock_cc:
226233
mock_cc.query.filter.return_value.count.return_value = 0
227-
settings_function_update(form)
234+
settings_function_update(form_del, form)
228235

229236
# The existing module file should have been overwritten (keyed by unique name, not upload filename)
230237
expected_file = os.path.join(custom_functions_dir, 'my_custom_function.py')
@@ -250,12 +257,12 @@ def test_update_different_filename_different_unique_name_fails(
250257
'function_name': 'Different Function',
251258
}
252259
"""
253-
file_storage = MockFileStorage('renamed_upload.py', different_content)
254-
form = make_update_form(VALID_FUNCTION_UNIQUE_NAME, file_storage)
260+
form_del = make_del_form(VALID_FUNCTION_UNIQUE_NAME)
261+
form = make_mod_form(MockFileStorage('renamed_upload.py', different_content))
255262

256263
with patch('mycodo.mycodo_flask.utils.utils_settings.INSTALL_DIRECTORY', tmp_install_dir), \
257264
patch('mycodo.mycodo_flask.utils.utils_settings.PATH_FUNCTIONS_CUSTOM', custom_functions_dir):
258-
settings_function_update(form)
265+
settings_function_update(form_del, form)
259266

260267
mock_popen.assert_not_called()
261268

@@ -269,13 +276,12 @@ def test_update_invalid_python_fails(
269276
original_content = VALID_FUNCTION_CONTENT + b'# original\n'
270277
write_existing_module(custom_functions_dir, original_content)
271278

272-
invalid_content = b'this is not valid python !@#$%'
273-
file_storage = MockFileStorage('my_custom_function.py', invalid_content)
274-
form = make_update_form(VALID_FUNCTION_UNIQUE_NAME, file_storage)
279+
form_del = make_del_form(VALID_FUNCTION_UNIQUE_NAME)
280+
form = make_mod_form(MockFileStorage('my_custom_function.py', b'this is not valid python !@#$%'))
275281

276282
with patch('mycodo.mycodo_flask.utils.utils_settings.INSTALL_DIRECTORY', tmp_install_dir), \
277283
patch('mycodo.mycodo_flask.utils.utils_settings.PATH_FUNCTIONS_CUSTOM', custom_functions_dir):
278-
settings_function_update(form)
284+
settings_function_update(form_del, form)
279285

280286
# Existing module should be untouched
281287
with open(os.path.join(custom_functions_dir, 'my_custom_function.py'), 'rb') as f:

0 commit comments

Comments
 (0)