From 8b1ca67d2247087a2cea5de138996bb305de275c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= Date: Sat, 30 Jun 2018 11:02:14 +0200 Subject: [PATCH] forms: revamp pages computation (#24706) --- tests/test_form_pages.py | 16 +++- wcs/backoffice/submission.py | 4 +- wcs/formdef.py | 35 ++------ wcs/forms/root.py | 166 ++++++++++++++++++----------------- 4 files changed, 109 insertions(+), 112 deletions(-) diff --git a/tests/test_form_pages.py b/tests/test_form_pages.py index a16346d8..c6b0a16a 100644 --- a/tests/test_form_pages.py +++ b/tests/test_form_pages.py @@ -518,7 +518,7 @@ def test_form_multi_page_condition_select(pub): condition={'type': 'python', 'value': 'var_foo == "Foo"'}), fields.PageField(id='3', label='3rd page', type='page', condition={'type': 'python', 'value': 'var_foo == "Bar"'}), - fields.StringField(id='3', label='string 2')] + fields.StringField(id='4', label='string 2')] formdef.store() formdef.data_class().wipe() resp = get_app(pub).get('/test/') @@ -743,7 +743,9 @@ def test_form_multi_page_condition_on_first_page(pub): condition={'type': 'python', 'value': 'False'}), fields.StringField(id='1', label='string'), fields.PageField(id='2', label='2nd page', type='page'), - fields.StringField(id='3', label='string 2')] + fields.StringField(id='3', label='string 2'), + fields.PageField(id='4', label='3rd page', type='page'), + ] formdef.store() resp = get_app(pub).get('/test/') formdef.data_class().wipe() @@ -751,10 +753,16 @@ def test_form_multi_page_condition_on_first_page(pub): with pytest.raises(AssertionError): resp.form.get('previous') resp.form['f3'] = 'foo' - resp = resp.form.submit('submit') + assert_current_page(resp, '2nd page') + resp = resp.form.submit('submit') # -> 3rd page + assert_current_page(resp, '3rd page') + resp = resp.form.submit('submit') # -> validation page assert 'Check values then click submit.' in resp.body assert resp.form['previous'] - resp = resp.form.submit('previous') + resp = resp.form.submit('previous') # -> 3rd page + assert_current_page(resp, '3rd page') + resp = resp.form.submit('previous') # -> 2nd page + assert_current_page(resp, '2nd page') assert resp.form['f3'] with pytest.raises(AssertionError): resp.form.get('previous') diff --git a/wcs/backoffice/submission.py b/wcs/backoffice/submission.py index 25ef519d..4e9f3115 100644 --- a/wcs/backoffice/submission.py +++ b/wcs/backoffice/submission.py @@ -156,7 +156,7 @@ class FormFillPage(PublicFormFillPage): form.add_hidden('submission_channel', self.selected_submission_channel) return form - def form_side(self, step_no, page_no, data=None, magictoken=None): + def form_side(self, step_no, page, data=None, magictoken=None): r = TemplateIO(html=True) get_response().filter['sidebar'] = self.get_sidebar(data) r += htmltext('
') @@ -167,7 +167,7 @@ class FormFillPage(PublicFormFillPage): r += htmltext('%s') % (draft_formdata_id, _('Delete this form')) r += htmltext('
') r += htmltext('
') - r += self.step(step_no, page_no, data=data) + r += self.step(step_no, page, data=data) r += htmltext('
') return mark_safe(str(r.getvalue())) diff --git a/wcs/formdef.py b/wcs/formdef.py index 0e894a9c..c151a7a6 100644 --- a/wcs/formdef.py +++ b/wcs/formdef.py @@ -439,25 +439,6 @@ class FormDef(StorableObject): return [] return [x.strip() for x in self.appearance_keywords.split()] - _start_page = None - def get_start_page(self): - if self._start_page is not None: - return self._start_page - start_page = 0 - if self.fields and self.fields[0].type == 'page' and self.fields[0].condition: - # multipage form with conditions on the first page - for field in self.fields: - if field.type != 'page': - continue - if field.is_visible(None, self): - break - start_page += 1 - else: - # not a single page :/ - return None - self._start_page = start_page - return self._start_page - def get_variable_options(self): variables = {} if not self.workflow.variables_formdef: @@ -523,7 +504,7 @@ class FormDef(StorableObject): def get_display_id_format(self): return '[formdef_id]-[form_number_raw]' - def create_form(self, page_no=0, displayed_fields=None): + def create_form(self, page=None, displayed_fields=None): form = Form(enctype="multipart/form-data", use_tokens=False) if self.appearance_keywords: form.attrs['class'] = 'quixote %s' % self.appearance_keywords @@ -532,20 +513,20 @@ class FormDef(StorableObject): form.ERROR_NOTICE = _('There were errors processing the form and ' 'you cannot go to the next page. Do ' 'check below that you filled all fields correctly.') - self.add_fields_to_form(form, page_no = page_no, displayed_fields = displayed_fields) + self.add_fields_to_form(form, page=page, displayed_fields=displayed_fields) return form - def add_fields_to_form(self, form, page_no = 0, displayed_fields = None, form_data = None): + def add_fields_to_form(self, form, page=None, displayed_fields=None, form_data=None): current_page = 0 + on_page = (page is None) for field in self.fields: if field.type == 'page': - if field is self.fields[0]: - continue - current_page += 1 - if current_page > page_no: + if on_page: break + if page.id == field.id: + on_page = True continue - if current_page != page_no: + if not on_page: continue if type(displayed_fields) is list: displayed_fields.append(field) diff --git a/wcs/forms/root.py b/wcs/forms/root.py index 198b11a7..7bbf0155 100644 --- a/wcs/forms/root.py +++ b/wcs/forms/root.py @@ -231,30 +231,26 @@ class FormPage(Directory, FormTemplateMixin): return True return False - def step(self, step_no, page_no, data=None): - if step_no == 0: - self.substvars['current_page_no'] = str(page_no + 1) + def step(self, step_no, current_page, data=None): get_logger().info('form %s - step %s' % (self.formdef.name, step_no)) page_labels = [] current_position = 1 - page_index = self.formdef.get_start_page() - for field in self.formdef.fields: - if field.type != 'page': - continue - if field.is_visible(data, self.formdef): - page_labels.append(field.label) - if page_index == page_no: - current_position = len(page_labels) - page_index += 1 - - if not page_labels: - page_labels.append(_('Filling')) + for i, page in enumerate(self.pages): + if page is None: # monopage form + page_labels.append(_('Filling')) + else: + page_labels.append(page.label) + if page is current_page: + current_position = i + 1 if step_no > 0: current_position = len(page_labels) + step_no + if step_no == 0: + self.substvars['current_page_no'] = current_position + if self.has_confirmation_page() and not self.edit_mode: page_labels.append(_('Validating')) @@ -282,16 +278,16 @@ class FormPage(Directory, FormTemplateMixin): r += htmltext('') return r.getvalue() - def page(self, page_no, page_change=True, page_error_messages=None): + def page(self, page, page_change=True, page_error_messages=None): displayed_fields = [] session = get_session() - if page_no > self.formdef.get_start_page(): + if page and self.pages.index(page) > 0: magictoken = get_request().form['magictoken'] self.feed_current_data(magictoken) - form = self.create_form(page_no, displayed_fields) + form = self.create_form(page, displayed_fields) if page_error_messages: form.add_global_errors(page_error_messages) if getattr(session, 'ajax_form_token', None): @@ -309,25 +305,25 @@ class FormPage(Directory, FormTemplateMixin): else: form_data = {} - if page_no == self.formdef.get_start_page() and not get_request().form.has_key('magictoken'): + if page == self.pages[0] and not get_request().form.has_key('magictoken'): magictoken = randbytes(8) else: magictoken = get_request().form['magictoken'] form.add_hidden('magictoken', magictoken) data = session.get_by_magictoken(magictoken, {}) - if page_no == self.formdef.get_start_page() and get_request().form.has_key('cancelurl'): + if page == self.pages[0] and get_request().form.has_key('cancelurl'): cancelurl = get_request().form['cancelurl'] session.add_magictoken(magictoken, {'__cancelurl': cancelurl}) - if self.edit_mode and page_no == self.page_number - 1: + if self.edit_mode and (page is None or page == self.pages[-1]): form.add_submit('submit', _('Save Changes')) - elif not self.has_confirmation_page() and page_no == self.page_number - 1: + elif not self.has_confirmation_page() and (page is None or page == self.pages[-1]): form.add_submit('submit', _('Submit')) else: form.add_submit('submit', _('Next')) - if page_no > self.formdef.get_start_page(): + if self.pages.index(page) > 0: form.add_submit('previous', _('Previous')) if page_change: @@ -394,7 +390,7 @@ class FormPage(Directory, FormTemplateMixin): self.html_top(self.formdef.name) form.add_hidden('step', '0') - form.add_hidden('page', page_no) + form.add_hidden('page', self.pages.index(page)) form.add_submit('cancel', _('Cancel'), css_class = 'cancel') if self.formdef.enable_tracking_codes and not self.edit_mode: @@ -404,8 +400,8 @@ class FormPage(Directory, FormTemplateMixin): context = { 'view': self, 'form': form, - 'form_side': lambda: self.form_side(0, page_no, data=data, magictoken=magictoken), - 'steps': lambda: self.step(0, page_no, data=data), + 'form_side': lambda: self.form_side(0, page, data=data, magictoken=magictoken), + 'steps': lambda: self.step(0, page, data=data), } if self.formdef.enable_tracking_codes and data: context['tracking_code_box'] = lambda: self.tracking_code_box(data, magictoken) @@ -414,7 +410,7 @@ class FormPage(Directory, FormTemplateMixin): list(self.get_formdef_template_variants(self.filling_templates)), context) - def form_side(self, step_no, page_no, data=None, magictoken=None): + def form_side(self, step_no, page, data=None, magictoken=None): '''Create the elements that typically appear aside the main form (tracking code and steps).''' r = TemplateIO(html=True) @@ -424,7 +420,7 @@ class FormPage(Directory, FormTemplateMixin): # data (e.g. the user is not on a insufficient authenticiation # context page) r += self.tracking_code_box(data, magictoken) - r += self.step(step_no, page_no, data=data) + r += self.step(step_no, page, data=data) r += htmltext(' ') return mark_safe(str(r.getvalue())) @@ -458,8 +454,10 @@ class FormPage(Directory, FormTemplateMixin): r += htmltext('') # return r.getvalue() - def feed_current_data(self, magictoken): - # create a fake FormData to feed variables + def get_transient_formdata(self, magictoken=Ellipsis): + if magictoken is Ellipsis: + magictoken = get_request().form.get('magictoken') + # create a fake FormData with current submission data formdata = FormData() formdata._formdef = self.formdef formdata.user = get_request().user @@ -469,7 +467,11 @@ class FormPage(Directory, FormTemplateMixin): draft_formdata_id = formdata.data.get('draft_formdata_id') if draft_formdata_id: formdata = self.formdef.data_class().get(draft_formdata_id) - formdata.status = str('') + formdata.status = '' + return formdata + + def feed_current_data(self, magictoken): + formdata = self.get_transient_formdata(magictoken) get_publisher().substitutions.feed(formdata) def check_disabled(self): @@ -495,7 +497,7 @@ class FormPage(Directory, FormTemplateMixin): self.html_top(self.formdef.name) r = TemplateIO(html=True) - r += self.form_side(step_no=0, page_no=self.formdef.get_start_page()) + r += self.form_side(step_no=0, page=self.pages[0]) auth_contexts = get_publisher().get_supported_authentication_contexts() r += htmltext('
') r += htmltext('

%s

') % _('You need a stronger authentication level to fill this form.') @@ -506,6 +508,27 @@ class FormPage(Directory, FormTemplateMixin): root_url, _('Login with %s') % auth_contexts[auth_context]) return r.getvalue() + _pages = None + @property + def pages(self): + if self._pages: + return self._pages + current_data = self.get_transient_formdata().data + pages = [] + field_page = None + for field in self.formdef.fields: + if field.type == 'page': + field_page = field + if field.is_visible(current_data, self.formdef): + pages.append(field) + if not field_page: # form without page fields + pages = [None] + self._pages = pages + return pages + + def reset_pages_cache(self): + self._pages = None + def _q_index(self): self.check_role() authentication_context_check_result = self.check_authentication_context() @@ -539,7 +562,7 @@ class FormPage(Directory, FormTemplateMixin): session_magic_token = session.get_by_magictoken( get_request().form.get('magictoken'), no_magic) if session_magic_token is no_magic: - if (get_request().form.get('page') != str(self.formdef.get_start_page()) or + if (get_request().form.get('page') != '0' or get_request().form.get('step') != '0'): # the magictoken that has been submitted is not available # in the session and we're not on the first page of the @@ -615,12 +638,12 @@ class FormPage(Directory, FormTemplateMixin): req.form['f%s' % field.id] = field.convert_value_to_str(data[field.id]) return self.validating(data) else: - page_no = self.formdef.get_start_page() - return self.page(page_no, True) + page_no = 0 + return self.page(self.pages[page_no], True) self.feed_current_data(None) - if self.formdef.get_start_page() is None: + if not self.pages: raise errors.TraversalError() - return self.page(self.formdef.get_start_page()) + return self.page(self.pages[0]) if form.get_submit() == 'cancel': get_logger().info('form %s - cancel' % (self.formdef.name)) @@ -660,7 +683,11 @@ class FormPage(Directory, FormTemplateMixin): try: page_no = int(form.get_widget('page').parse()) except TypeError: - page_no = -1 + # this situation shouldn't arise (that likely means the + # page hidden field had an error in its submission), in + # that case we just fall back to the first page. + page_no = 0 + page = self.pages[page_no] try: magictoken = form.get_widget('magictoken').parse() except KeyError: @@ -668,13 +695,13 @@ class FormPage(Directory, FormTemplateMixin): self.feed_current_data(magictoken) - form = self.create_form(page_no) + form = self.create_form(page=page) form.add_submit('previous') if self.formdef.enable_tracking_codes: form.add_submit('removedraft') form.add_submit('savedraft') form.add_submit('submit') - if page_no > self.formdef.get_start_page() and form.get_submit() == 'previous': + if page_no > 0 and form.get_submit() == 'previous': return self.previous_page(page_no, magictoken) if self.formdef.enable_tracking_codes and form.get_submit() == 'removedraft': @@ -688,13 +715,8 @@ class FormPage(Directory, FormTemplateMixin): return redirect(filled.get_url().rstrip('/')) page_error_messages = [] - if form.get_submit() == 'submit': - pages = [x for x in self.formdef.fields if x.type == 'page'] - try: - page = pages[page_no] - post_conditions = page.post_conditions or [] - except IndexError: - post_conditions = [] + if form.get_submit() == 'submit' and page: + post_conditions = page.post_conditions or [] form_data = session.get_by_magictoken(magictoken, {}) data = self.formdef.get_data(form) form_data.update(data) @@ -717,12 +739,7 @@ class FormPage(Directory, FormTemplateMixin): # by clicking on a submit widget; for example if an "add row" # button is clicked. if form.has_errors() or form.get_submit() is True: - if page_no == -1: - # this situation shouldn't arise (that likely means the - # page hidden field had an error in its submission), in - # that case we just fall back to the first page. - page_no = self.formdef.get_start_page() - return self.page(page_no, page_change=False, + return self.page(page, page_change=False, page_error_messages=page_error_messages) form_data = session.get_by_magictoken(magictoken, {}) @@ -731,14 +748,7 @@ class FormPage(Directory, FormTemplateMixin): session.add_magictoken(magictoken, form_data) - while True: - page_no = int(page_no) + 1 - try: - next_page = self.formdef.get_page(page_no) - except IndexError: - break - if next_page.is_visible(form_data, self.formdef): - break + page_no += 1 draft_id = session.get_by_magictoken(magictoken, {}).get('draft_formdata_id') if draft_id: @@ -756,7 +766,12 @@ class FormPage(Directory, FormTemplateMixin): # if there's no draft yet and tracking codes are enabled, create one filled = self.save_draft(form_data, page_no) - if page_no == self.page_number: + # the page has been successfully submitted, maybe new pages + # should be revealed. + self.feed_current_data(magictoken) + self.reset_pages_cache() + + if int(page_no) == len(self.pages): # last page has been submitted req = get_request() for field in self.formdef.fields: @@ -784,14 +799,14 @@ class FormPage(Directory, FormTemplateMixin): form.add_submit('savedraft') else: - return self.page(page_no) + return self.page(self.pages[page_no]) if step == 1: form.add_submit('previous') magictoken = form.get_widget('magictoken').parse() if form.get_submit() == 'previous': - return self.previous_page(self.page_number, magictoken) + return self.previous_page(len(self.pages), magictoken) magictoken = form.get_widget('magictoken').parse() form_data = session.get_by_magictoken(magictoken, {}) data = self.formdef.get_data(form) @@ -812,7 +827,7 @@ class FormPage(Directory, FormTemplateMixin): form_data = session.get_by_magictoken(magictoken, {}) if form.get_submit() == 'previous': - return self.previous_page(self.page_number, magictoken) + return self.previous_page(len(self.pages), magictoken) if self.formdef.enable_tracking_codes and form.get_submit() == 'removedraft': return self.removedraft() @@ -841,19 +856,12 @@ class FormPage(Directory, FormTemplateMixin): session = get_session() form_data = session.get_by_magictoken(magictoken, {}) - while True: - page_no = int(page_no) - 1 - try: - previous_page = self.formdef.get_page(page_no) - except IndexError: - break - if not previous_page.condition: - break - - if previous_page.is_visible(form_data, self.formdef): - break + try: + previous_page = self.pages[int(page_no - 1)] + except IndexError: + previous_page = self.pages[0] - return self.page(page_no, page_change=True) + return self.page(previous_page, page_change=True) def removedraft(self): magictoken = get_request().form.get('magictoken') @@ -912,7 +920,7 @@ class FormPage(Directory, FormTemplateMixin): if not form_data: return result_error('missing data') - form = self.create_form(page_no) + form = self.create_form(page=self.pages[page_no]) data = self.formdef.get_data(form) if not data: return result_error('nothing to save') @@ -1102,7 +1110,7 @@ class FormPage(Directory, FormTemplateMixin): context = { 'view': self, 'form': form, - 'form_side': lambda: self.form_side(step_no=1, page_no=None, data=data, + 'form_side': lambda: self.form_side(step_no=1, page=None, data=data, magictoken=magictoken), } -- 2.18.0