[PATCH 3 of 3 saem] [seda] Fix crash when profile has an originating agency not tight to an authority record

Philippe Pepiot philippe.pepiot at logilab.fr
Tue May 23 10:11:43 CEST 2017


On 05/19/2017 10:51 AM, Sylvain Thenault wrote:
> # HG changeset patch
> # User Sylvain Thénault <sylvain.thenault at logilab.fr>
> # Date 1495116472 -7200
> #      Thu May 18 16:07:52 2017 +0200
> # Node ID 792626340c7a0c3d74208dddff9be1116f8dcbc4
> # Parent  b619a152b407fb8352f7748c53a74df134ff85e4
> # Available At http://hg.logilab.org/review/cubes/saem_ref
> #              hg pull http://hg.logilab.org/review/cubes/saem_ref -r 792626340c7a
> [seda] Fix crash when profile has an originating agency not tight to an authority record
>
> it used to fail with 'None has no attribute ark' because monkey-patched method
> doesn't handle the case where .agency is None (properly handled in seda's
> original implementation).
>
> Closes extranet #22071759
>
> diff --git a/cubicweb_saem_ref/entities/seda.py b/cubicweb_saem_ref/entities/seda.py
> --- a/cubicweb_saem_ref/entities/seda.py
> +++ b/cubicweb_saem_ref/entities/seda.py
> @@ -51,11 +51,11 @@ def xsd_archival_agreement(self, parent,
>
>
>  # override agency_id to return ark instead of eid
>  @monkeypatch(SEDA1XSDExport)
>  def agency_id(self, agency):
> -    return agency.agency.ark
> +    return agency.agency.ark if agency.agency else None

Applied first two patches.

Could this be more clear with 'if agency.agency is not None' ?

Also the original implementation in cubicweb-seda suggest that agency 
itself could be None, maybe you should handle this here too ?

>
>
>  SEDAArchiveTransferIClonableAdapter.__select__ &= is_in_state('published')
>  SEDAArchiveTransferIClonableAdapter.rtype = 'new_version_of'
>  SEDAArchiveTransferIClonableAdapter.skiprtypes += ('clone_of',)
> diff --git a/test/test_seda.py b/test/test_seda.py
> --- a/test/test_seda.py
> +++ b/test/test_seda.py
> @@ -77,17 +77,22 @@ class SEDAExportTC(CubicWebTC):
>                  self.assertIn('ArchivalProfile', xml)
>                  self.assertIn(transfer.ark, xml)
>
>      def test_include_agency_ark(self):
>          with self.admin_access.client_cnx() as cnx:
> -            record = testutils.authority_record(cnx, u'DGSI')
> -
>              transfer = testutils.seda_transfer(cnx)
>              _, _, unit_alt_seq = testutils.create_archive_unit(transfer)
> -            cnx.create_entity('SEDAOriginatingAgency', seda_originating_agency_from=unit_alt_seq,
> -                              seda_originating_agency_to=record)
> +            agency = cnx.create_entity('SEDAOriginatingAgency',
> +                                       seda_originating_agency_from=unit_alt_seq)
>
> +            # ensure it doesn't crash if agency is not tight to an authority record
> +            for xml in self.iter_seda_xsd(transfer):
> +                self.assertIn('OriginatingAgency', xml)
> +
> +            # ensure we get the ark as agency identifier
> +            record = testutils.authority_record(cnx, u'DGSI')
> +            agency.cw_set(seda_originating_agency_to=record)
>              for xml in self.iter_seda_xsd(transfer):
>                  self.assertIn(record.ark, xml)
>
>
>  if __name__ == '__main__':
>



More information about the saem-devel mailing list