formdef: add internal identifier attribute, separated from url_name (#15663)
This will allow changing url name behaviour without impacting on storage matters.
This commit is contained in:
parent
bac624f1d2
commit
c3be61b8d1
|
|
@ -353,7 +353,8 @@ def test_forms_edit(pub):
|
|||
assert resp.location == 'http://example.net/backoffice/forms/1/'
|
||||
resp = resp.follow()
|
||||
assert FormDef.get(1).name == 'new title'
|
||||
assert FormDef.get(1).url_name == 'new-title'
|
||||
assert FormDef.get(1).url_name == 'form-title'
|
||||
assert FormDef.get(1).internal_identifier == 'new-title'
|
||||
|
||||
def test_form_category(pub):
|
||||
create_superuser(pub)
|
||||
|
|
@ -770,8 +771,8 @@ def test_form_import(pub):
|
|||
resp = resp.forms[0].submit()
|
||||
assert FormDef.count() == 1
|
||||
|
||||
# import the same formdef a second time, make sure the urlname is not
|
||||
# reused
|
||||
# import the same formdef a second time, make sure url name and internal
|
||||
# identifier are not reused
|
||||
resp = app.get('/backoffice/forms/')
|
||||
resp = resp.click(href='import')
|
||||
resp.forms[0]['file'] = Upload('formdef.wcs', formdef_xml)
|
||||
|
|
@ -779,9 +780,11 @@ def test_form_import(pub):
|
|||
assert FormDef.count() == 2
|
||||
assert FormDef.get(1).url_name == 'form-title'
|
||||
assert FormDef.get(2).url_name == 'form-title-1'
|
||||
assert FormDef.get(1).internal_identifier == 'form-title'
|
||||
assert FormDef.get(2).internal_identifier == 'form-title-1'
|
||||
|
||||
# import a formdef with an url_name that doesn't match its title,
|
||||
# it should be updated to match.
|
||||
# import a formdef with an url name that doesn't match its title,
|
||||
# it should be kept intact.
|
||||
formdef.url_name = 'xxx-other-form-title'
|
||||
formdef_xml = ET.tostring(formdef.export_to_xml(include_id=True))
|
||||
|
||||
|
|
@ -789,7 +792,8 @@ def test_form_import(pub):
|
|||
resp = resp.click(href='import')
|
||||
resp.forms[0]['file'] = Upload('formdef.wcs', formdef_xml)
|
||||
resp = resp.forms[0].submit()
|
||||
assert FormDef.get(3).url_name == 'form-title-2'
|
||||
assert FormDef.get(3).url_name == 'xxx-other-form-title'
|
||||
assert FormDef.get(3).internal_identifier == 'form-title-2'
|
||||
|
||||
# import an invalid file
|
||||
resp = app.get('/backoffice/forms/')
|
||||
|
|
@ -1443,7 +1447,7 @@ def test_form_overwrite(pub):
|
|||
|
||||
app = login(get_app(pub))
|
||||
resp = app.get('/backoffice/forms/%s/' % formdef_id)
|
||||
resp = resp.click(href='overwrite')
|
||||
resp = resp.click(href='overwrite', index=0)
|
||||
resp.forms[0]['file'] = Upload('formdef.wcs', formdef_xml)
|
||||
resp = resp.forms[0].submit()
|
||||
assert 'The form removes and changes fields' in resp.body
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import cPickle
|
||||
import datetime
|
||||
import json
|
||||
import sys
|
||||
|
|
@ -9,24 +10,31 @@ import pytest
|
|||
from quixote import cleanup
|
||||
from wcs import formdef
|
||||
from wcs.formdef import FormDef
|
||||
from wcs.qommon.http_request import HTTPRequest
|
||||
from wcs.workflows import Workflow
|
||||
from wcs.fields import StringField, FileField, DateField, ItemField
|
||||
|
||||
from utilities import create_temporary_pub
|
||||
from utilities import create_temporary_pub, clean_temporary_pub
|
||||
|
||||
def setup_module(module):
|
||||
cleanup()
|
||||
def pytest_generate_tests(metafunc):
|
||||
if 'pub' in metafunc.fixturenames:
|
||||
metafunc.parametrize('pub', ['pickle', 'sql'], indirect=True)
|
||||
|
||||
global pub
|
||||
|
||||
pub = create_temporary_pub()
|
||||
@pytest.fixture
|
||||
def pub(request):
|
||||
pub = create_temporary_pub(sql_mode=(request.param == 'sql'))
|
||||
req = HTTPRequest(None, {'SCRIPT_NAME': '/', 'SERVER_NAME': 'example.net'})
|
||||
pub.set_app_dir(req)
|
||||
pub.cfg['language'] = {'language': 'en'}
|
||||
pub.write_cfg()
|
||||
|
||||
return pub
|
||||
|
||||
def teardown_module(module):
|
||||
shutil.rmtree(pub.APP_DIR)
|
||||
clean_temporary_pub()
|
||||
|
||||
|
||||
def test_is_disabled():
|
||||
def test_is_disabled(pub):
|
||||
formdef = FormDef()
|
||||
assert not formdef.is_disabled()
|
||||
|
||||
|
|
@ -34,7 +42,7 @@ def test_is_disabled():
|
|||
assert formdef.is_disabled()
|
||||
|
||||
|
||||
def test_is_disabled_publication_date():
|
||||
def test_is_disabled_publication_date(pub):
|
||||
formdef = FormDef()
|
||||
|
||||
formdef.publication_date = '%s-%02d-%02d' % (datetime.datetime.today() - datetime.timedelta(1)).timetuple()[:3]
|
||||
|
|
@ -44,7 +52,7 @@ def test_is_disabled_publication_date():
|
|||
assert formdef.is_disabled()
|
||||
|
||||
|
||||
def test_is_disabled_expiration_date():
|
||||
def test_is_disabled_expiration_date(pub):
|
||||
formdef = FormDef()
|
||||
|
||||
formdef.expiration_date = '%s-%02d-%02d' % (datetime.datetime.today() - datetime.timedelta(1)).timetuple()[:3]
|
||||
|
|
@ -54,7 +62,7 @@ def test_is_disabled_expiration_date():
|
|||
assert not formdef.is_disabled()
|
||||
|
||||
|
||||
def test_is_disabled_publication_datetime():
|
||||
def test_is_disabled_publication_datetime(pub):
|
||||
formdef = FormDef()
|
||||
|
||||
formdef.publication_date = '%s-%02d-%02d %02d:%02d' % (
|
||||
|
|
@ -66,7 +74,7 @@ def test_is_disabled_publication_datetime():
|
|||
assert formdef.is_disabled()
|
||||
|
||||
|
||||
def test_is_disabled_expiration_datetime():
|
||||
def test_is_disabled_expiration_datetime(pub):
|
||||
formdef = FormDef()
|
||||
|
||||
formdef.expiration_date = '%s-%02d-%02d %02d:%02d' % (
|
||||
|
|
@ -77,26 +85,28 @@ def test_is_disabled_expiration_datetime():
|
|||
datetime.datetime.now() + datetime.timedelta(hours=1)).timetuple()[:5]
|
||||
assert not formdef.is_disabled()
|
||||
|
||||
def test_title_change():
|
||||
def test_title_change(pub):
|
||||
formdef = FormDef()
|
||||
formdef.name = 'foo'
|
||||
formdef.store()
|
||||
assert FormDef.get(formdef.id).name == 'foo'
|
||||
assert FormDef.get(formdef.id).url_name == 'foo'
|
||||
assert FormDef.get(formdef.id).internal_identifier == 'foo'
|
||||
|
||||
formdef.name = 'bar'
|
||||
formdef.store()
|
||||
assert FormDef.get(formdef.id).name == 'bar'
|
||||
assert FormDef.get(formdef.id).url_name == 'bar'
|
||||
assert FormDef.get(formdef.id).url_name == 'foo'
|
||||
assert FormDef.get(formdef.id).internal_identifier == 'bar'
|
||||
|
||||
# makes sure the url_name doesn't change if there are submitted forms
|
||||
# makes sure the internal_name doesn't change if there are submitted forms
|
||||
formdef.data_class()().store()
|
||||
formdef.name = 'baz'
|
||||
formdef.store()
|
||||
assert FormDef.get(formdef.id).name == 'baz'
|
||||
assert FormDef.get(formdef.id).url_name == 'bar' # didn't change
|
||||
assert FormDef.get(formdef.id).internal_identifier == 'bar' # didn't change
|
||||
|
||||
def test_substitution_variables():
|
||||
def test_substitution_variables(pub):
|
||||
formdef = FormDef()
|
||||
formdef.name = 'foo'
|
||||
formdef.store()
|
||||
|
|
@ -121,7 +131,7 @@ def test_substitution_variables():
|
|||
assert formdef.get_substitution_variables()['form_option_bar'] == 'Bar'
|
||||
assert formdef.get_substitution_variables()['form_option_bar_raw'] == 'bar'
|
||||
|
||||
def test_schema_with_date_variable():
|
||||
def test_schema_with_date_variable(pub):
|
||||
FormDef.wipe()
|
||||
formdef = FormDef()
|
||||
formdef.name = 'foo'
|
||||
|
|
@ -137,7 +147,7 @@ def test_schema_with_date_variable():
|
|||
formdef.workflow_options = {'foo': time.gmtime(time.mktime((2016, 4, 2, 0, 0, 0, 0, 0, 0)))}
|
||||
assert json.loads(formdef.export_to_json())['options']['foo'].startswith('2016-04')
|
||||
|
||||
def test_substitution_variables_object():
|
||||
def test_substitution_variables_object(pub):
|
||||
formdef = FormDef()
|
||||
formdef.name = 'foo'
|
||||
formdef.store()
|
||||
|
|
@ -157,7 +167,7 @@ def test_substitution_variables_object():
|
|||
with pytest.raises(AttributeError):
|
||||
assert substs.foobar
|
||||
|
||||
def test_file_field_migration():
|
||||
def test_file_field_migration(pub):
|
||||
pub.cfg['filetypes'] = {1:
|
||||
{'mimetypes': [
|
||||
'application/pdf',
|
||||
|
|
@ -185,3 +195,19 @@ def test_file_field_migration():
|
|||
assert formdef.fields[0].document_type['mimetypes'] == ['image/*', 'application/pdf,application/vnd.oasis.opendocument.text,application/msword,application/vnd.openxmlformats-officedocument.wordprocessingml.document,application/vnd.oasis.opendocument.spreadsheet,application/vnd.ms-excel,application/vnd.openxmlformats-officedocument.spreadsheetml.sheet']
|
||||
assert formdef.fields[1].document_type['label'] == 'Image files'
|
||||
assert formdef.fields[0].document_type['label'] == 'Image files, Documents'
|
||||
|
||||
def test_internal_identifier_migration(pub):
|
||||
FormDef.wipe()
|
||||
formdef = FormDef()
|
||||
formdef.name = 'foo'
|
||||
formdef.fields = []
|
||||
formdef.store()
|
||||
|
||||
obj = cPickle.load(open(formdef.get_object_filename()))
|
||||
del obj.internal_identifier
|
||||
cPickle.dump(obj, open(formdef.get_object_filename(), 'w'))
|
||||
assert cPickle.load(open(formdef.get_object_filename())).internal_identifier is None
|
||||
assert FormDef.get(formdef.id, ignore_migration=True).internal_identifier is None
|
||||
|
||||
formdef = FormDef.get(formdef.id)
|
||||
assert formdef.internal_identifier == 'foo'
|
||||
|
|
|
|||
|
|
@ -569,7 +569,7 @@ def test_urlname_change():
|
|||
|
||||
formdef.name = 'tests2'
|
||||
formdef.store()
|
||||
assert formdef.url_name == 'tests2'
|
||||
assert formdef.url_name == 'tests'
|
||||
|
||||
formdef.name = 'tests'
|
||||
formdef.store()
|
||||
|
|
|
|||
|
|
@ -949,8 +949,9 @@ class FormDefPage(Directory):
|
|||
return redirect('.')
|
||||
|
||||
def overwrite_by_formdef(self, new_formdef):
|
||||
# keep current formdef id, url_name and sql table name
|
||||
# keep current formdef id, url_name, internal identifier and sql table name
|
||||
new_formdef.id = self.formdef.id
|
||||
new_formdef.internal_identifier = self.formdef.internal_identifier
|
||||
new_formdef.url_name = self.formdef.url_name
|
||||
new_formdef.table_name = self.formdef.table_name
|
||||
# keep currently assigned category and workflow
|
||||
|
|
@ -1552,7 +1553,7 @@ class FormsDirectory(AccessControlled, Directory):
|
|||
form.set_error('file', msg)
|
||||
raise ValueError()
|
||||
|
||||
formdef.url_name = None # a new one will be set in .store()
|
||||
formdef.internal_identifier = None # a new one will be set in .store()
|
||||
formdef.disabled = True
|
||||
formdef.store()
|
||||
get_session().message = ('info',
|
||||
|
|
|
|||
|
|
@ -69,6 +69,7 @@ class FormDef(StorableObject):
|
|||
description = None
|
||||
keywords = None
|
||||
url_name = None
|
||||
internal_identifier = None # mostly for pickle
|
||||
table_name = None # for SQL only
|
||||
fields = None
|
||||
category_id = None
|
||||
|
|
@ -101,7 +102,7 @@ class FormDef(StorableObject):
|
|||
|
||||
# declarations for serialization
|
||||
TEXT_ATTRIBUTES = ['name', 'url_name', 'description', 'keywords',
|
||||
'publication_date', 'expiration_date',
|
||||
'publication_date', 'expiration_date', 'internal_identifier',
|
||||
'disabled_redirection',]
|
||||
BOOLEAN_ATTRIBUTES = ['discussion', 'detailed_emails', 'disabled',
|
||||
'only_allow_one', 'enable_tracking_codes', 'confirmation',
|
||||
|
|
@ -204,6 +205,10 @@ class FormDef(StorableObject):
|
|||
changed = True
|
||||
break
|
||||
|
||||
if not self.internal_identifier:
|
||||
self.internal_identifier = self.url_name
|
||||
changed = True
|
||||
|
||||
for f in self.fields or []:
|
||||
changed |= f.migrate()
|
||||
|
||||
|
|
@ -238,7 +243,7 @@ class FormDef(StorableObject):
|
|||
actions = sql.do_formdef_tables(self)
|
||||
else:
|
||||
cls = new.classobj(self.url_name.title(), (FormData,),
|
||||
{'_names': 'form-%s' % self.url_name,
|
||||
{'_names': 'form-%s' % self.internal_identifier,
|
||||
'_formdef': self})
|
||||
actions = []
|
||||
setattr(sys.modules['formdef'], self.url_name.title(), cls)
|
||||
|
|
@ -264,14 +269,29 @@ class FormDef(StorableObject):
|
|||
suffix_no = 0
|
||||
while True:
|
||||
try:
|
||||
formdef = self.get_by_urlname(new_url_name, ignore_migration=True)
|
||||
obj = self.get_on_index(new_url_name, 'url_name', ignore_migration=True)
|
||||
except KeyError:
|
||||
break
|
||||
if obj.id == self.id:
|
||||
break
|
||||
suffix_no += 1
|
||||
new_url_name = '%s-%s' % (base_new_url_name, suffix_no)
|
||||
return new_url_name
|
||||
|
||||
def get_new_internal_identifier(self):
|
||||
new_internal_identifier = simplify(self.name)
|
||||
base_new_internal_identifier = new_internal_identifier
|
||||
suffix_no = 0
|
||||
while True:
|
||||
try:
|
||||
formdef = self.get_by_urlname(new_internal_identifier, ignore_migration=True)
|
||||
except KeyError:
|
||||
break
|
||||
if formdef.id == self.id:
|
||||
break
|
||||
suffix_no += 1
|
||||
new_url_name = '%s-%s' % (base_new_url_name, suffix_no)
|
||||
return new_url_name
|
||||
new_internal_identifier = '%s-%s' % (base_new_internal_identifier, suffix_no)
|
||||
return new_internal_identifier
|
||||
|
||||
@classmethod
|
||||
def get_new_id(cls, create=False):
|
||||
|
|
@ -303,16 +323,18 @@ class FormDef(StorableObject):
|
|||
sql.formdef_wipe()
|
||||
|
||||
def store(self):
|
||||
new_url_name = self.get_new_url_name()
|
||||
if not self.url_name:
|
||||
self.url_name = new_url_name
|
||||
if new_url_name != self.url_name:
|
||||
# title changed, url will be changed only if the formdef is
|
||||
# currently being imported (self.id is None) or if there are not
|
||||
# yet any submitted forms
|
||||
data_class = self.data_class()
|
||||
if self.id is None or data_class().count() == 0:
|
||||
self.url_name = new_url_name
|
||||
if self.url_name is None:
|
||||
# set url name if it's not yet there
|
||||
self.url_name = self.get_new_url_name()
|
||||
new_internal_identifier = self.get_new_internal_identifier()
|
||||
if not self.internal_identifier:
|
||||
self.internal_identifier = new_internal_identifier
|
||||
if new_internal_identifier != self.internal_identifier:
|
||||
# title changed, internal identifier will be changed only if
|
||||
# the formdef is currently being imported (self.id is None)
|
||||
# or if there are not yet any submitted forms
|
||||
if self.id is None or self.data_class().count() == 0:
|
||||
self.internal_identifier = new_internal_identifier
|
||||
self.last_modification_time = time.localtime()
|
||||
if get_request() and get_request().user:
|
||||
self.last_modification_user_id = str(get_request().user.id)
|
||||
|
|
@ -846,6 +868,14 @@ class FormDef(StorableObject):
|
|||
formdef = cls.import_from_xml_tree(tree, charset=charset,
|
||||
include_id=include_id)
|
||||
|
||||
if formdef.url_name:
|
||||
try:
|
||||
obj = cls.get_on_index(formdef.url_name, 'url_name', ignore_migration=True)
|
||||
except KeyError:
|
||||
pass
|
||||
else:
|
||||
formdef.url_name = formdef.get_new_url_name()
|
||||
|
||||
# fix max_field_id if necessary
|
||||
if formdef.max_field_id is not None:
|
||||
max_field_id = max([lax_int(x.id) for x in formdef.fields])
|
||||
|
|
|
|||
|
|
@ -762,7 +762,7 @@ def drop_global_views(conn, cur):
|
|||
def do_global_views(conn, cur):
|
||||
# recreate global views
|
||||
from wcs.formdef import FormDef
|
||||
view_names = [get_formdef_view_name(x) for x in FormDef.select()]
|
||||
view_names = [get_formdef_view_name(x) for x in FormDef.select(ignore_migration=True)]
|
||||
|
||||
cur.execute('''SELECT table_name FROM information_schema.views
|
||||
WHERE table_schema = 'public'
|
||||
|
|
|
|||
Loading…
Reference in New Issue