ISSUE-161: AMI CSV export fails when an ADO has a missing type key

[aksm]

Resolves #161.

[DiegoPino]

@aksm. Since type key is one of two keys required (label/type) for an AMI set. When no type key is set, how would an AMI set from an export work? Could you please elaborate on the solution here? Thanks!

[aksm]

@DiegoPino Sorry, that was silly of me. Will convert to draft and continue working on this.

[aksm]

@DiegoPino So is the idea to log it and return an empty row? throw an exception? something else?

[DiegoPino]

I don’t know, on one side the fix is the isset, but then after that, What would your suggestion be? An ami set without type can not be created or will be wrong. Message? Skip the row? Or generate only the csv but without an ami set + a message? What would you expect as a user?

On Mon, Jun 12, 2023 at 8:11 PM Albert Min @.***> wrote:

@DiegoPino https://github.com/DiegoPino So is the idea to log it and return an empty row? throw an exception? something else?

— Reply to this email directly, view it on GitHub https://github.com/esmero/ami/pull/168#issuecomment-1587833761, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABU7ZZZCYLN3EEUE5JX4KPTXK5LW5ANCNFSM6AAAAAAY576EF4 . You are receiving this because you were mentioned.Message ID: @.***>

-- Diego Pino Navarro Digital Repositories Developer Metropolitan New York Library Council (METRO)

[aksm]

Hmm, maybe add an option to the next step to skip the row or generate the CSV only? As a user I'd want to make the decision beforehand.

I don’t know, on one side the fix is the isset, but then after that, What would your suggestion be? An ami set without type can not be created or will be wrong. Message? Skip the row? Or generate only the csv but without an ami set + a message? What would you expect as a user? On Mon, Jun 12, 2023 at 8:11 PM Albert Min @.> wrote: @DiegoPino https://github.com/DiegoPino So is the idea to log it and return an empty row? throw an exception? something else? — Reply to this email directly, view it on GitHub <#168 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABU7ZZZCYLN3EEUE5JX4KPTXK5LW5ANCNFSM6AAAAAAY576EF4 . You are receiving this because you were mentioned.Message ID: @.> -- Diego Pino Navarro Digital Repositories Developer Metropolitan New York Library Council (METRO)

[DiegoPino]

Ok. That makes sense. But, we need to be sure its marked as an exceptional case. Ados should always have a type key

On Mon, Jun 12, 2023 at 9:01 PM Albert Min @.***> wrote:

Hmm, maybe add an option to the next step to skip the row or generate the CSV only? As a user I'd want to make the decision beforehand.

I don’t know, on one side the fix is the isset, but then after that, What would your suggestion be? An ami set without type can not be created or will be wrong. Message? Skip the row? Or generate only the csv but without an ami set + a message? What would you expect as a user? On Mon, Jun 12, 2023 at 8:11 PM Albert Min @.> wrote: @DiegoPino https://github.com/DiegoPino https://github.com/DiegoPino https://github.com/DiegoPino So is the idea to log it and return an empty row? throw an exception? something else? — Reply to this email directly, view it on GitHub <#168 (comment) https://github.com/esmero/ami/pull/168#issuecomment-1587833761>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABU7ZZZCYLN3EEUE5JX4KPTXK5LW5ANCNFSM6AAAAAAY576EF4 https://github.com/notifications/unsubscribe-auth/ABU7ZZZCYLN3EEUE5JX4KPTXK5LW5ANCNFSM6AAAAAAY576EF4 . You are receiving this because you were mentioned.Message ID: @.> -- Diego Pino Navarro Digital Repositories Developer Metropolitan New York Library Council (METRO)

— Reply to this email directly, view it on GitHub https://github.com/esmero/ami/pull/168#issuecomment-1587908435, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABU7ZZ6HFKPFDJFWFCOOEJ3XK5RRDANCNFSM6AAAAAAY576EF4 . You are receiving this because you were mentioned.Message ID: @.***>

-- Diego Pino Navarro Digital Repositories Developer Metropolitan New York Library Council (METRO)

[aksm]

Hi @DiegoPino, does this seem like a reasonable way to go about this?

[DiegoPino]

@aksm did you test this? Could you provide information on how this was tested? Thanks

[aksm]

@DiegoPino Yes, I tested by selecting different combinations of ADOs, e.g.:

• 1 ADO with no type key
• Multiple ADOs that included one without the type key
• Multiple ADOs all with type keys

Then I processed with and without the new AMI set option to see that it properly produced the CSV and skipped the AMI set if there's a type missing on any ADOs and gave the user a message.

@aksm did you test this? Could you provide information on how this was tested? Thanks