[PATCH eac] [schema] record_id is mandatory on AuthorityRecord

Frank Bessou frank.bessou at logilab.fr
Thu Aug 29 17:22:11 CEST 2019



On 29/08/2019 16:55, Guillaume Vandevelde wrote:
> # HG changeset patch
> # User Katia Saurfelt <katia.saurfelt at logilab.fr>
> # Date 1562946486 -7200
> #      Fri Jul 12 17:48:06 2019 +0200
> # Node ID 2d712be68b27aaa4fbccf8e66ae4f4f29a8d0df9
> # Parent  a28e00e0b1b980dc730739b9dd308e831e089978
> # Available At http://hg.logilab.org/review/cubes/eac
> #              hg pull http://hg.logilab.org/review/cubes/eac -r 2d712be68b27
> [schema] record_id is mandatory on AuthorityRecord
> 
> diff -r a28e00e0b1b9 -r 2d712be68b27 cubicweb_eac/dataimport.py
> --- a/cubicweb_eac/dataimport.py	Tue Jul 30 19:20:31 2019 +0200
> +++ b/cubicweb_eac/dataimport.py	Fri Jul 12 17:48:06 2019 +0200
> @@ -1041,14 +1041,12 @@
>           """Parse the `control` tag."""
>           record_id = self._elem_find(control, 'eac:recordId')
>           if record_id is not None and record_id.text and record_id.text.strip():

You can gain an indentation level here:

if record_id is None or ... :
     raise InvalidEAC('recordId element in control tag is mandatory')
self.record.extid = 'auth...

otherwise, LGTM :)

> -            self.record.extid = 'authorityrecord-{}'.format(record_id.text.strip())
> -            self.record.values['record_id'] = set([to_unicode(record_id.text)])
> +            record_id = record_id.text.strip()
> +            self.record.extid = 'authorityrecord-{}'.format(record_id)
> +            self.record.values['record_id'] = set([to_unicode(record_id)])
>               self.record_visited(record_id, self.record)
>           else:
> -            self.record.extid = self._gen_extid()
> -            self.import_log.record_warning(self._(
> -                'found no recordId element in control tag, using %s as cwuri') %
> -                self.record.extid, line=control.sourceline)
> +            raise InvalidEAC('recordId element in control tag is mandatory')
>           for other_record_id in self._elem_findall(control, 'eac:otherRecordId'):
>               other_id = other_record_id.text.strip()
>               if other_id:
> diff -r a28e00e0b1b9 -r 2d712be68b27 cubicweb_eac/migration/0.9.0_Any.py
> --- a/cubicweb_eac/migration/0.9.0_Any.py	Tue Jul 30 19:20:31 2019 +0200
> +++ b/cubicweb_eac/migration/0.9.0_Any.py	Fri Jul 12 17:48:06 2019 +0200
> @@ -49,9 +49,11 @@
>   for etype in ('GeneralContext', 'Mandate', 'Occupation', 'History',
>                 'AgentFunction', 'LegalStatus', 'AgentPlace', 'Structure'):
>       add_attribute(etype, 'items')
> -
> +
>   add_relation_type('place_entry_relation')
>   
>   add_attribute('Activity', 'agent_type')
>   
>   add_attribute('AuthorityRecord', 'languages')
> +
> +sync_schema_props_perms('record_id')
> diff -r a28e00e0b1b9 -r 2d712be68b27 cubicweb_eac/schema.py
> --- a/cubicweb_eac/schema.py	Tue Jul 30 19:20:31 2019 +0200
> +++ b/cubicweb_eac/schema.py	Fri Jul 12 17:48:06 2019 +0200
> @@ -57,7 +57,7 @@
>       start_date = Date(constraints=[BoundaryConstraint(
>           '<=', Attribute('end_date'), msg=_('start date must be less than end date'))])
>       end_date = Date()
> -    record_id = String(indexed=True)
> +    record_id = String(indexed=True, required=True, unique=True)
>       isni = String(unique=True,
>                     description=_('International Standard Name Identifier'))
>       languages = String()
> diff -r a28e00e0b1b9 -r 2d712be68b27 cubicweb_eac/testutils.py
> --- a/cubicweb_eac/testutils.py	Tue Jul 30 19:20:31 2019 +0200
> +++ b/cubicweb_eac/testutils.py	Fri Jul 12 17:48:06 2019 +0200
> @@ -27,13 +27,14 @@
>   from cubicweb.dataimport.importer import SimpleImportLog
>   
>   
> -def authority_record(cnx, name, kind=u'person', **kwargs):
> +def authority_record(cnx, record_id, name, kind=u'person', **kwargs):
>       """Return an AuthorityRecord with specified kind and name."""
>       kind_eid = cnx.find('AgentKind', name=kind)[0][0]
>       if 'reverse_name_entry_for' not in kwargs:
>           kwargs['reverse_name_entry_for'] = cnx.create_entity(
>               'NameEntry', parts=name, form_variant=u'authorized')
> -    return cnx.create_entity('AuthorityRecord', agent_kind=kind_eid, **kwargs)
> +    return cnx.create_entity('AuthorityRecord', record_id=record_id,
> +                             agent_kind=kind_eid, **kwargs)
>   
>   
>   def eac_import(cnx, fpath):
> diff -r a28e00e0b1b9 -r 2d712be68b27 test/export-simple.rst
> --- a/test/export-simple.rst	Tue Jul 30 19:20:31 2019 +0200
> +++ b/test/export-simple.rst	Fri Jul 12 17:48:06 2019 +0200
> @@ -4,7 +4,7 @@
>   .. code-block:: python
>   
>       >>> from cubicweb import Binary
> -    >>> record = testutils.authority_record(cnx, u'Charlie', record_id=u'666')
> +    >>> record = testutils.authority_record(cnx, u'666', u'Charlie')
>       >>> home_addr = cnx.create_entity(
>       ...     'PostalAddress', street=u'Place du Capitole',
>       ...     postalcode=u'31000', city=u'Toulouse')
> @@ -25,7 +25,7 @@
>       ...     resource_relation_agent=record,
>       ...     xml_wrap=Binary('<pif><paf>pouf</paf></pif>'))
>       >>> record2 = testutils.authority_record(
> -    ...     cnx, u'does not matter', kind=u'authority',
> +    ...     cnx, u'2', u'does not matter', kind=u'authority',
>       ...     cwuri=u'http://www.example.org/record2')
>       >>> chronological_relation = cnx.create_entity(
>       ...     'ChronologicalRelation', entry=u'Super Service',
> diff -r a28e00e0b1b9 -r 2d712be68b27 test/test_dataimport.py
> --- a/test/test_dataimport.py	Tue Jul 30 19:20:31 2019 +0200
> +++ b/test/test_dataimport.py	Fri Jul 12 17:48:06 2019 +0200
> @@ -646,6 +646,7 @@
>           self.assertEqual(not_visited,
>                            {'maintenanceStatus': set([12]),
>                             'publicationStatus': set([14]),
> +                          'recordId': set([8]),
>                             'maintenanceAgency': set([16]),
>                             'languageDeclaration': set([21]),
>                             'languageUsed': set([172, 179]),
> diff -r a28e00e0b1b9 -r 2d712be68b27 test/test_entities.py
> --- a/test/test_entities.py	Tue Jul 30 19:20:31 2019 +0200
> +++ b/test/test_entities.py	Fri Jul 12 17:48:06 2019 +0200
> @@ -29,7 +29,7 @@
>   
>       def test_richstring_plain(self):
>           with self.admin_access.cnx() as cnx:
> -            alice = testutils.authority_record(cnx, u'Alice')
> +            alice = testutils.authority_record(cnx, u'T-01', u'Alice')
>               desc = u'ding\nlikes rabbits'
>               mandate = cnx.create_entity(
>                   'Mandate', term=u'ding-girl',
> @@ -46,7 +46,7 @@
>   
>       def test_richstring_html_simple(self):
>           with self.admin_access.cnx() as cnx:
> -            alice = testutils.authority_record(cnx, u'Alice')
> +            alice = testutils.authority_record(cnx, u'T-01', u'Alice')
>               desc = u'<span>ding</span>'
>               mandate = cnx.create_entity(
>                   'Mandate', term=u'ding-girl',
> @@ -61,7 +61,7 @@
>   
>       def test_richstring_html_multiple_elements(self):
>           with self.admin_access.cnx() as cnx:
> -            alice = testutils.authority_record(cnx, u'Alice')
> +            alice = testutils.authority_record(cnx, u'T-01', u'Alice')
>               desc = [u'<h1>she <i>rules!</i></h1>', u'<a href="1">pif</a>']
>               mandate = cnx.create_entity(
>                   'Mandate', term=u'chairgirl',
> @@ -78,7 +78,7 @@
>   
>       def test_richstring_markdown(self):
>           with self.admin_access.cnx() as cnx:
> -            alice = testutils.authority_record(cnx, u'Alice')
> +            alice = testutils.authority_record(cnx, u'T-01', u'Alice')
>               desc = u'[pif](http://gadget.com) is *red*'
>               desc_html = (
>                   u'<a href="http://gadget.com">pif</a> '
> @@ -97,7 +97,7 @@
>   
>       def test_richstring_rest(self):
>           with self.admin_access.cnx() as cnx:
> -            alice = testutils.authority_record(cnx, u'Alice')
> +            alice = testutils.authority_record(cnx, u'T-01', u'Alice')
>               desc = u'`pif <http://gadget.com>`_ is *red*'
>               desc_html = (
>                   u'<a class="reference" href="http://gadget.com">pif</a> '
> @@ -122,7 +122,7 @@
>               self.assertEqual(res, [])
>   
>           with self.admin_access.cnx() as cnx:
> -            alice = testutils.authority_record(cnx, u'Alice')
> +            alice = testutils.authority_record(cnx, u'T-01', u'Alice')
>               mandate = cnx.create_entity(
>                   'Mandate', term=u'w',
>                   description=None,
> @@ -139,7 +139,7 @@
>   
>       def test_richstring_non_none_non_html(self):
>           with self.admin_access.cnx() as cnx:
> -            arecord = testutils.authority_record(cnx, u'R')
> +            arecord = testutils.authority_record(cnx, u'R-01', u'R')
>               mandate = cnx.create_entity(
>                   'Mandate', term=u'w',
>                   description=u' ',
> diff -r a28e00e0b1b9 -r 2d712be68b27 test/test_hooks.py
> --- a/test/test_hooks.py	Tue Jul 30 19:20:31 2019 +0200
> +++ b/test/test_hooks.py	Fri Jul 12 17:48:06 2019 +0200
> @@ -9,9 +9,9 @@
>   
>       def setup_database(self):
>           with self.admin_access.cnx() as cnx:
> -            self.arecord1_eid = testutils.authority_record(cnx, u'alice').eid
> -            self.arecord2_eid = testutils.authority_record(cnx, u'bob').eid
> -            self.arecord3_eid = testutils.authority_record(cnx, u'charles').eid
> +            self.arecord1_eid = testutils.authority_record(cnx, u'T-01', u'alice').eid
> +            self.arecord2_eid = testutils.authority_record(cnx, u'T-02', u'bob').eid
> +            self.arecord3_eid = testutils.authority_record(cnx, u'T-03', u'charles').eid
>               cnx.create_entity('AssociationRelation',
>                                 entry=u'alice and bob are friends',
>                                 association_from=self.arecord1_eid,
> diff -r a28e00e0b1b9 -r 2d712be68b27 test/test_schema.py
> --- a/test/test_schema.py	Tue Jul 30 19:20:31 2019 +0200
> +++ b/test/test_schema.py	Fri Jul 12 17:48:06 2019 +0200
> @@ -43,7 +43,7 @@
>           context).
>           """
>           with self.admin_access.cnx() as cnx:
> -            ar = testutils.authority_record(cnx, u'test')
> +            ar = testutils.authority_record(cnx, u'T-04', u'test')
>               cnx.create_entity('AgentPlace',
>                                 place_agent=ar,
>                                 place_address=cnx.create_entity('PostalAddress'))
> @@ -56,7 +56,7 @@
>           """
>           with self.admin_access.cnx() as cnx:
>               with self.assertValidationError(cnx) as cm:
> -                testutils.authority_record(cnx, u'Arthur',
> +                testutils.authority_record(cnx, u'T-04', u'Arthur',
>                                              start_date=date(524, 2, 9),
>                                              end_date=date(500, 7, 12))
>               self.assertIn("must be less than", str(cm.exception))
> @@ -70,7 +70,7 @@
>               # hence the constraint is only triggered on start_date modification
>               self.skipTest('unsupported sqlite version')
>           with self.admin_access.cnx() as cnx:
> -            agent = testutils.authority_record(cnx, u'Arthur',
> +            agent = testutils.authority_record(cnx, u'T-04', u'Arthur',
>                                                  start_date=date(454, 2, 9),
>                                                  end_date=date(475, 4, 12))
>               cnx.commit()
> @@ -85,7 +85,7 @@
>               ValidationError expected
>           """
>           with self.admin_access.cnx() as cnx:
> -            agent = testutils.authority_record(cnx, u'Arthur', end_date=date(476, 2, 9))
> +            agent = testutils.authority_record(cnx, u'T-04', u'Arthur', end_date=date(476, 2, 9))
>               cnx.commit()
>               with self.assertValidationError(cnx) as cm:
>                   agent.cw_set(start_date=date(527, 4, 12))
> @@ -179,7 +179,7 @@
>       def test_agent_kind_relation(self):
>           """Test we can only change kind from unknown to another."""
>           with self.admin_access.cnx() as cnx:
> -            record = testutils.authority_record(cnx, u'bob', kind=u'unknown-agent-kind')
> +            record = testutils.authority_record(cnx, u'T-02', u'bob', kind=u'unknown-agent-kind')
>               cnx.commit()
>               record.cw_set(agent_kind=cnx.find('AgentKind', name=u'person').one())
>               cnx.commit()
> @@ -190,13 +190,15 @@
>           with self.admin_access.cnx() as cnx:
>               self.create_user(cnx, login=u'toto', groups=('users', 'guests'))
>               function = cnx.create_entity('AgentFunction', name=u'director')
> -            testutils.authority_record(cnx, u'admin record', reverse_function_agent=function)
> +            testutils.authority_record(cnx, u'T-00', u'admin record',
> +                                       reverse_function_agent=function)
>               cnx.commit()
>   
>           with self.new_access('toto').cnx() as cnx:
>               # user can create and modify its own records
>               function = cnx.create_entity('AgentFunction', name=u'grouillot')
> -            record = testutils.authority_record(cnx, u'bob', reverse_function_agent=function)
> +            record = testutils.authority_record(cnx, u'T-02', u'bob',
> +                                                reverse_function_agent=function)
>               cnx.commit()
>               function.cw_set(name=u'grouyo')
>               cnx.commit()
> diff -r a28e00e0b1b9 -r 2d712be68b27 test/test_views.py
> --- a/test/test_views.py	Tue Jul 30 19:20:31 2019 +0200
> +++ b/test/test_views.py	Fri Jul 12 17:48:06 2019 +0200
> @@ -49,7 +49,7 @@
>           fname = 'FRAD033_EAC_00001_simplified.xml'
>           with self.admin_access.client_cnx() as cnx:
>               # ISNI is the same as the agent in EAC file.
> -            testutils.authority_record(cnx, u'bob', isni=u'22330001300016')
> +            testutils.authority_record(cnx, u'T-02', u'bob', isni=u'22330001300016')
>               cnx.commit()
>           with self.admin_access.web_request() as req:
>               # simply test the form properly render and is well formed
> @@ -86,7 +86,7 @@
>       def test_export_filename(self):
>           with self.admin_access.web_request() as req:
>               cnx = req.cnx
> -            record = testutils.authority_record(cnx, u'jim')
> +            record = testutils.authority_record(cnx, u'T-07', u'jim')
>               for isni, expected_filename in (
>                   (u"", "EAC_{0}.xml".format(record.eid)),
>                   (u"ZZZ/4242", "EAC_ZZZ_4242.xml".format(record.eid)),
> @@ -101,7 +101,7 @@
>   
>       def test_xmlwrap_component(self):
>           with self.admin_access.cnx() as cnx:
> -            bob = testutils.authority_record(cnx, u'bob')
> +            bob = testutils.authority_record(cnx, u'T-02', u'bob')
>               uri = cnx.create_entity('ExternalUri', uri=u'http://logilab.fr')
>               cnx.create_entity('EACResourceRelation',
>                                 resource_relation_agent=bob,
> @@ -122,7 +122,7 @@
>   
>       def test_no_copy_action(self):
>           with self.admin_access.cnx() as cnx:
> -            testutils.authority_record(cnx, u'toto')
> +            testutils.authority_record(cnx, u'T-06', u'toto')
>               cnx.commit()
>           with self.admin_access.web_request() as req:
>               rset = req.execute('Any X WHERE N name_entry_for X, N parts "toto"')
> @@ -132,8 +132,8 @@
>       def test_unrelated_authorityrecord(self):
>           from cubicweb_eac.views import unrelated_authorityrecord
>           with self.admin_access.cnx() as cnx:
> -            ar1 = testutils.authority_record(cnx, u'1').eid
> -            ar2 = testutils.authority_record(cnx, u'2').eid
> +            ar1 = testutils.authority_record(cnx, u'T-1', u'1').eid
> +            ar2 = testutils.authority_record(cnx, u'T-2', u'2').eid
>               # ExternalUri should not appear in choices.
>               cnx.create_entity('ExternalUri', uri=u'http://no-where')
>               cnx.commit()
> 

-- 
Frank Bessou
Logilab         https://www.logilab.fr



More information about the saem-devel mailing list