metanorma / pubid-iec

PubID spec and implementation for IEC deliverables
BSD 2-Clause "Simplified" License
2 stars 0 forks source link

Missing stage_codes in stages.yaml #112

Closed opoudjis closed 11 months ago

opoudjis commented 1 year ago

pubid-core-1.8.0/lib/pubid/core/harmonized_stage_code.rb

    14|       @stages = if stage_or_code.is_a?(Array)
    15|                   stage_or_code
    16|                 elsif stage_or_code.is_a?(String) && config.stages["codes_description"].key?(stage_or_code)
    17|                   [stage_or_code]
    18|                   # when stage is stage name
=>  19|                 elsif config.stages["stage_codes"].key?(stage_or_code.to_s)
    20|                   ["#{config.stages["stage_codes"][stage_or_code.to_s]}.#{config.stages["substage_codes"][substage.to_s]}"]

If the pubid flavour gem is initialised without stage_codes in its YAML, as is the case with pubid-iec/stages.yaml, this step crashes.

@mico These gems are not being tested adequately.

Either add stage_codes to pubid-iec, or else make harmonized_stage_code not presuppose the existence of stage_codes: !config.stages("stage_codes", stage_or_code.to_s).nil? The latter is the safest.

mico commented 1 year ago

@opoudjis could you help me with use case where we face this bug?

mico commented 1 year ago

@opoudjis I released pubid-core update (https://github.com/metanorma/pubid-core/releases/tag/v1.8.1) implementing what you was asking here, but it might not help with parsing stages that are not defined in config.stages["codes_description"] and it will raise Pubid::Core::Errors::HarmonizedStageCodeInvalidError for unknown stages like "60.92".

ronaldtse commented 1 year ago

@mico which stage codes do not have harmonize stage descriptions? Thanks.

mico commented 1 year ago

@mico which stage codes do not have harmonize stage descriptions? Thanks.

@ronaldtse from conversation with @opoudjis I learned that "60.92" don't have stage description

ronaldtse commented 1 year ago

I think we need to implement a list that says what stages are not allowed: https://www.iso.org/files/live/sites/isoorg/files/developing_standards/docs/en/stage_codes.pdf

All these empty boxes should not be allowed to be used to create stages:

Screenshot 2023-06-24 at 10 40 54 AM

If someone attempts to use those stages, a HarmonizedStageCodeInvalidError should definitely be thrown.

mico commented 1 year ago

@opoudjis could you comment on "60.92" code you found in IEEE identifiers?

ronaldtse commented 1 year ago

@opoudjis please help clarify -- this code 60.92 is not meant to exist?

opoudjis commented 1 year ago

it was in a sample document header created by @Intelligent2013 as an example, top of https://github.com/metanorma/metanorma-iec/issues/154

Alex, to confirm, that was an example, not a real state, right?

Intelligent2013 commented 1 year ago

@opoudjis when I've worked on the issue https://github.com/metanorma/metanorma-iec/issues/153, just for debug purposes I've get the sample document from mn-samples-iec (https://raw.githubusercontent.com/metanorma/mn-samples-iec/main/sources/iec-rice-en.adoc) with

:docstage: 30
:docsubstage: 92

and update some fields for ISO-IEC-TR-63306-1-2020 (also changed to :docstage: 60), therefore we have:

:docstage: 60
:docsubstage: 92

So, you can ignore it.

opoudjis commented 11 months ago

Just as a reminder:

ronaldtse commented 11 months ago

@mico can you help answer this? Thanks.

mico commented 11 months ago

Just as a reminder:

  • If you get a request with an illegal stage/substage, and there are no stage codes defined in YAML, you should still not crash as the code currently does, but degrade gracefully. This is a problem with pubid-core, and it will turn up elsewhere.
  • I need pubid-iec as a source of truth to tell me whether a stage/substage in IEC is legal or not. Will test @mico's code update when I have free time.

@opoudjis @ronaldtse An error will be raised if the code is not defined in codes_description in https://github.com/metanorma/pubid-iec/blob/main/stages.yaml

ronaldtse commented 11 months ago

@mico whats the error that we can catch for that? Thanks.

mico commented 11 months ago

@mico whats the error that we can catch for that? Thanks.

https://github.com/metanorma/pubid-core/blob/527e40d9103c56f9beed1185b0bc4dacc4322401/lib/pubid/core/harmonized_stage_code.rb#L31

@ronaldtse should I document it in pubid-core README or somewhere else?

ronaldtse commented 11 months ago

@mico yes please document it in the pubid-core README, thanks!

opoudjis commented 11 months ago

Btw, already using that error in Metanorma to identify extraneous stages.

ronaldtse commented 11 months ago

Thanks! Is this issue resolved now?

ronaldtse commented 11 months ago

Ping @opoudjis