[PATCH eac] Correct bug happening on History child appending

Frank Bessou frank.bessou at logilab.fr
Wed Aug 28 17:57:55 CEST 2019


Applied, thanks :)

On 28/08/2019 16:26, Guillaume Vandevelde wrote:
> # HG changeset patch
> # User Guillaume Vandevelde <gvandevelde at logilab.fr>
> # Date 1562835700 -7200
> #      Thu Jul 11 11:01:40 2019 +0200
> # Node ID 5d5b47eefb0768503682f9fd6e990c3294e60d0b
> # Parent  1bc51d853cdcc18d221f4c668adb5c6f5f89c50f
> # Available At http://hg.logilab.org/review/cubes/eac
> #              hg pull http://hg.logilab.org/review/cubes/eac -r 5d5b47eefb07
> Correct bug happening on History child appending
> 
> Correct a child appending bug that appeared when the function was modified for better handling the dates. When the child appending was more than 1 depth like for
> History -> Event -> DateEntity
> the extid was appended to the grandparent entity and the parent.
> 
> diff -r 1bc51d853cdc -r 5d5b47eefb07 cubicweb_eac/dataimport.py
> --- a/cubicweb_eac/dataimport.py	Wed Aug 28 12:21:44 2019 +0200
> +++ b/cubicweb_eac/dataimport.py	Thu Jul 11 11:01:40 2019 +0200
> @@ -192,7 +192,7 @@
>   
>   
>   add_citations_for = partial(add_child_for, relation='has_citation', builder='build_citation')
> -add_events_for = partial(add_child_for, relation='has_event', builder='build_event')
> +add_events_for = partial(add_child_for, relation='has_event', builder='parse_history_chronitems')
>   add_names_for = partial(add_child_for, relation='simple_name_relation',
>                           builder='build_name_child')
>   add_dates_for = partial(add_child_for, relation='date_relation',
> @@ -688,7 +688,6 @@
>               yield ExtEntity('Citation', self._gen_extid(), values)
>   
>       @relate_to_record_through('History', 'history_agent')
> -    @add_events_for('History')
>       @add_citations_for('History')
>       @add_items_for('History')
>       @elem_maybe_none
> @@ -701,7 +700,19 @@
>                         'text_format': set([desc_format])}
>               if abstract is not None and abstract.text:
>                   values['abstract'] = set([text_type(abstract.text)])
> -            yield ExtEntity('History', self._gen_extid(), values)
> +            history = ExtEntity('History', self._gen_extid(), values)
> +            for child in self.parse_history_chronitems(elem):
> +                if child.etype == 'HistoricalEvent':
> +                    history.values.setdefault(
> +                        'has_event', set()).add(child.extid)
> +                yield child
> +            yield history
> +
> +    def parse_history_chronitems(self, elem):
> +        """Build en `Event` external entity."""
> +        for citem in self._elem_findall(elem, './/eac:chronItem'):
> +            for extentity in self.build_event(citem):
> +                yield extentity
>   
>       @add_dates_for('HistoricalEvent')
>       @add_place_entries_for('HistoricalEvent')
> @@ -710,14 +721,11 @@
>       @elem_maybe_none
>       def build_event(self, elem):
>           """Build a `HistoricalEvent` external entity."""
> -        citems = self._elem_findall(elem, './/eac:chronItem')
> -        if citems is not None:
> -            for citem in citems:
> -                values = {}
> -                event = self._elem_find(citem, 'eac:event')
> -                if event is not None and event.text:
> -                    values['event'] = set([text_type(event.text)])
> -                yield ExtEntity('HistoricalEvent', self._gen_extid(), values)
> +        values = {}
> +        event = self._elem_find(elem, 'eac:event')
> +        if event is not None and event.text:
> +            values['event'] = set([text_type(event.text)])
> +        yield ExtEntity('HistoricalEvent', self._gen_extid(), values)
>   
>       @elem_maybe_none
>       @relate_to_record_through('Structure', 'structure_agent')
> diff -r 1bc51d853cdc -r 5d5b47eefb07 test/test_dataimport.py
> --- a/test/test_dataimport.py	Wed Aug 28 12:21:44 2019 +0200
> +++ b/test/test_dataimport.py	Thu Jul 11 11:01:40 2019 +0200
> @@ -287,8 +287,8 @@
>                ),
>               ('History', _gen_extid(),
>                {'abstract': set([u'Test of an abstract element']),
> -              'has_citation': set(['33', '32']),
> -              'has_event': set(['34', '35']),
> +              'has_citation': set(['37', '38']),
> +              'has_event': set(['32', '34']),
>                 'text': set(["\n".join((
>                     u'<p xmlns="urn:isbn:1-931666-33-4" '
>                     u'xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" '
> @@ -311,6 +311,27 @@
>                 'history_agent': set(['authorityrecord-FRAD033_EAC_00001']),
>                 },
>                ),
> +            ('HistoricalEvent', _gen_extid(),
> +             {'date_relation': set(['33']),
> +              'event': [u'Left Mer and moved to the mainland.\n\t      '
> +                        u'Worked at various jobs including canecutter\n\t      '
> +                        u'and railway labourer.\n\t      '],
> +              },
> +             ),
> +            ('DateEntity', _gen_extid(),
> +             {'end_date': [datetime.date(1957, 1, 1)],
> +              'start_date': [datetime.date(1957, 1, 1)]}
> +             ),
> +            ('HistoricalEvent', _gen_extid(),
> +             {'date_relation': set(['35']),
> +              'event': set([u'Union representative, Townsville-\n\t      '
> +                            u'Mount Isa rail construction project.\n\t      ']),
> +              },
> +             ),
> +            ('DateEntity', _gen_extid(),
> +             {'end_date': [datetime.date(1961, 1, 1)],
> +              'start_date': [datetime.date(1960, 1, 1)]}
> +             ),
>               ('Citation', _gen_extid(),
>                {'uri': set(['http://www.assemblee-nationale.fr/histoire/images-decentralisation/'
>                             'decentralisation/loi-du-22-decembre-1789-.pdf'])},
> @@ -318,17 +339,6 @@
>               ('Citation', _gen_extid(),
>                {'uri': set(['http://pifgadget']), 'note': set(['Voir aussi pifgadget'])},
>                ),
> -            ('HistoricalEvent', _gen_extid(),
> -             {'event': [u'Left Mer and moved to the mainland.\n\t      '
> -                        u'Worked at various jobs including canecutter\n\t      '
> -                        u'and railway labourer.\n\t      '],
> -              },
> -             ),
> -            ('HistoricalEvent', _gen_extid(),
> -             {'event': set([u'Union representative, Townsville-\n\t      '
> -                            u'Mount Isa rail construction project.\n\t      ']),
> -              },
> -             ),
>               ('Structure', _gen_extid(),
>                {'description': set([u'<p xmlns="urn:isbn:1-931666-33-4" '
>                                     u'xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" '
> @@ -369,11 +379,11 @@
>                ),
>               ('Occupation', _gen_extid(),
>                {'term': set([u'Réunioniste']),
> -              'date_relation': set(['42']),
> +              'date_relation': set(['44']),
>                 'description': set([u'Organisation des réunions ...']),
>                 'description_format': set([u'text/plain']),
>                 'occupation_agent': set(['authorityrecord-FRAD033_EAC_00001']),
> -              'has_citation': set(['43']),
> +              'has_citation': set(['45']),
>                 'equivalent_concept': set(['http://pifgadget.com']),
>                 },
>                ),
> @@ -391,7 +401,7 @@
>                                 u'xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" '
>                                 u'xmlns:xlink="http://www.w3.org/1999/xlink">very famous</p>']),
>                 'content_format': set([u'text/html']),
> -              'has_citation': set(['45']),
> +              'has_citation': set(['47']),
>                 'general_context_of': set(['authorityrecord-FRAD033_EAC_00001']),
>                 }
>                ),
> @@ -407,7 +417,7 @@
>               ('HierarchicalRelation', _gen_extid(),
>                {'entry': set([u"Gironde. Conseil général. Direction de l'administration et de "
>                               u"la sécurité juridique"]),
> -              'date_relation': set(['47']),
> +              'date_relation': set(['49']),
>                 'description': set([u'Coucou']),
>                 'description_format': set([u'text/plain']),
>                 'hierarchical_parent': set(['CG33-DIRADSJ']),
> @@ -432,7 +442,7 @@
>               ('ChronologicalRelation', _gen_extid(),
>                {'chronological_predecessor': set(['whatever']),
>                 'chronological_successor': set(['authorityrecord-FRAD033_EAC_00001']),
> -              'date_relation': set(['49']),
> +              'date_relation': set(['51']),
>                 'entry': set([u'CG32']),
>                 },
>                ),
> @@ -444,7 +454,7 @@
>               ('ChronologicalRelation', _gen_extid(),
>                {'chronological_predecessor': set(['authorityrecord-FRAD033_EAC_00001']),
>                 'chronological_successor': set(['/dev/null']),
> -              'date_relation': set(['51']),
> +              'date_relation': set(['53']),
>                 'xml_wrap': set(['<gloups xmlns="urn:isbn:1-931666-33-4"'
>                                  u' xmlns:xsi="http://www.w3.org/2001/XML'
>                                  u'Schema-instance" xmlns:xlink="http://'
> @@ -463,7 +473,7 @@
>                ),
>               ('EACResourceRelation', _gen_extid(),
>                {'agent_role': set([u'creatorOf']),
> -              'date_relation': set(['54']),
> +              'date_relation': set(['56']),
>                 'xml_attributes': set([u'{"{http://www.w3.org/1999/xlink}actuate": "onRequest", '
>                                        u'"{http://www.w3.org/1999/xlink}show": "new", '
>                                        u'"{http://www.w3.org/1999/xlink}type": "simple"}']),
> @@ -525,7 +535,7 @@
>                                     'and abolishment of schools.\n\t  </p>']),
>                 'r_type': set([u'controls']),
>                 'description_format': set([u'text/html']),
> -              'date_relation': set(['57']),
> +              'date_relation': set(['59']),
>                 'relation_entry': set([u'Establishment and abolishment\n\tof schools\n\t']),
>                 'xml_attributes': set([u'{}'])
>                 },
> @@ -546,7 +556,7 @@
>                 'description_format': set([u'text/html']),
>                 'relation_entry': set([u'Some relation entry\n          ']),
>                 'xml_attributes': set([u'{}']),
> -              'date_relation': ['59'],
> +              'date_relation': ['61'],
>                 },
>                ),
>               ('DateEntity', _gen_extid(),
> @@ -565,7 +575,7 @@
>                 'description_format': set([u'text/html']),
>                 'relation_entry': set([u'Some relation entry\n          ']),
>                 'xml_attributes': set([u'{}']),
> -              'date_relation': ['61'],
> +              'date_relation': ['63'],
>                 },
>                ),
>               ('DateEntity', _gen_extid(),
> @@ -784,7 +794,7 @@
>                                  cwuri=u'http://data.culture.fr/thesaurus/page/ark:/67717/T1-1074')
>               cnx.commit()
>               created, updated = testutils.eac_import(cnx, fpath)
> -            self.assertEqual(len(created), 71)
> +            self.assertEqual(len(created), 73)
>               self.assertEqual(updated, set())
>               rset = cnx.find('AuthorityRecord', isni=u'22330001300016')
>               self.assertEqual(len(rset), 1)
> 

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



More information about the saem-devel mailing list