[EDOI] Add new object types EDOI #681

i506210 · 2025-01-27T12:18:39Z

No description provided.

cla-assistant · 2025-01-27T12:18:53Z

All committers have signed the CLA.

cla-assistant · 2025-01-27T12:18:57Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

schneidermic0 · 2025-01-28T08:15:40Z

Hi @i506210,

Thank you for your contribution.

Would you mind to split this pull request (PR) into two PRs? One for EDOI and one for EDOT. I believe the handling for one PR per object type will be easier.

Furthermore, you might want to remove the .DS_Store files from the PR.

Thanks :)

i506210 · 2025-01-28T09:18:05Z

Hi @i506210,

Thank you for your contribution.

Would you mind to split this pull request (PR) into two PRs? One for EDOI and one for EDOT. I believe the handling for one PR per object type will be easier.

Furthermore, you might want to remove the .DS_Store files from the PR.

Thanks :)

Hi @schneidermic0,

I have split Pull request into 2 and triggered PR. could you please check

Thanks & REgards,
Venkat

Markus1812

Hi Venkat,

thanks for the update. I've added some questions/comments to your pull request.

Please note, that for the title and description, we use title case and sentence case respectively. I've not marked every occurrence but if you could please adjust that.

"! <p class="shorttext">Interface Version</p>    "<- title (title case)
"! Interface version                             "<- description (sentence case)

Markus1812 · 2025-01-28T13:04:31Z

file-formats/edoi/type/zif_aff_edoi_v1.intf.abap

+      "! <p class="shorttext">eDocument Interface</p>
+      "! eDocument Interface
+      "! $required
+      edocument_interface_id      TYPE c LENGTH 30,
+      "! <p class="shorttext">eDocument Interface Description</p>
+      "! Interface Description.
+      "! $required
+      interface_description       TYPE c LENGTH 100,


Is this edocument_interface_id and interface_description the name and description of this EDOI object?

To prevent redundant data, the name is saved in the file name, and the description is saved in ty_main->header (where you currently have selected a header with description length 60, but there is also a version for length 80 or length 100 available).

May be I might have added 'header' by referring the sample interface. I don't know the purpose of 'header' section. If its not necessary, i will remove it.
Could you please explain the statement "the name is saved in the file name,", As I didn't find the field 'file name'.

May be I might have added 'header' by referring the sample interface. I don't know the purpose of 'header' section. If its not necessary, i will remove it.

We expect formatVersionand header as mandatory fields. See https://github.com/SAP/abap-file-formats/blob/main/docs/json.md#writing-json-schema-with-abap-types

Therefore, please do not remove it ;)

Could you please explain the statement "the name is saved in the file name,", As I didn't find the field 'file name'.

If you check the file names in AFF, the object name is stored there. There is no need to add it to the file content. This would be redundant data.

corrected, removed object name and description from general information

file-formats/edoi/type/zif_aff_edoi_v1.intf.abap

i506210 · 2025-01-28T19:58:34Z

Hi Venkat,

thanks for the update. I've added some questions/comments to your pull request.

Please note, that for the title and description, we use title case and sentence case respectively. I've not marked every occurrence but if you could please adjust that.
"! <p class="shorttext">Interface Version</p>    "<- title (title case)
"! Interface version                             "<- description (sentence case)

Hi

Hi Venkat,

thanks for the update. I've added some questions/comments to your pull request.

Please note, that for the title and description, we use title case and sentence case respectively. I've not marked every occurrence but if you could please adjust that.
"! <p class="shorttext">Interface Version</p>    "<- title (title case)
"! Interface version                             "<- description (sentence case)

Hi Michael,

I corrected all the places.

Thanks & Regards,
Venkat

file-formats/edoi/type/zif_aff_edoi_v1.intf.abap

GuilhermeSaraiva96 · 2025-02-19T13:46:25Z

file-formats/edoi/type/zif_aff_edoi_v1.intf.abap

+  "! eDocument interface versions
+  TYPES ty_edoc_int_drc_attributes TYPE SORTED TABLE OF ty_edoc_int_drc_attr WITH UNIQUE KEY process_type.
+
+  CONSTANTS:


Could you move this block closer to ty_int_direction? This is where this enumeration is used

GuilhermeSaraiva96 · 2025-02-19T13:48:52Z

file-formats/edoi/type/zif_aff_edoi_v1.intf.abap

+      "! $required
+      interface_version             TYPE c LENGTH 4,
+      "! <p class="shorttext">Interface Version Description</p>
+      "! Interface dersion description


Suggested change

"! Interface dersion description

"! Interface version description

GuilhermeSaraiva96 · 2025-02-19T13:57:20Z

file-formats/edoi/type/zif_aff_edoi_v1.intf.abap

+      "! <p class="shorttext">Interface Version</p>
+      "! Interface version
+      "! $required
+      interface_version             TYPE c LENGTH 4,


What are the possible values for the interface version here? Are they fixed values and should it be an enumeration as well?

GuilhermeSaraiva96 · 2025-02-19T13:59:06Z

file-formats/edoi/type/zif_aff_edoi_v1.intf.abap

+  TYPES:
+    "! <p class="shorttext">Attribute Determination for Communication with Cloud Edition</p>
+    "! Attribute determination for communication with cloud edition
+    BEGIN OF ty_edoc_int_drc_attr,


Suggested change

BEGIN OF ty_edoc_int_drc_attr,

BEGIN OF ty_determination_attribute,

GuilhermeSaraiva96 · 2025-02-19T14:04:15Z

file-formats/edoi/type/zif_aff_edoi_v1.intf.abap

+    END OF ty_edoc_int_drc_attr.
+  "! <p class="shorttext">Interface Versions</p>
+  "! eDocument interface versions
+  TYPES ty_edoc_int_drc_attributes TYPE SORTED TABLE OF ty_edoc_int_drc_attr WITH UNIQUE KEY process_type.


Suggested change

TYPES ty_edoc_int_drc_attributes TYPE SORTED TABLE OF ty_edoc_int_drc_attr WITH UNIQUE KEY process_type.

TYPES ty_determination_attribute TYPE SORTED TABLE OF ty_edoc_int_drc_attr WITH UNIQUE KEY process_type.

I would say that having edoc and interface in the name of the variable is unnecessary as the whole object type is about the edoc interface. What do you think?

When you will be using the AFF, it will be something like this, the easier the name, the better
edoi_model-edoc_spec_additional_table[index]

GuilhermeSaraiva96 · 2025-02-19T14:09:28Z

file-formats/edoi/type/zif_aff_edoi_v1.intf.abap

+      "! <p class="shorttext">Attribute Determination for Communication with Cloud Edition</p>
+      "! Attribute determination for communication with cloud edition
+      "! $required
+      edoc_spec_additional_table TYPE ty_edoc_int_drc_attributes,


is this name correct? doesn't seem to have much in common with the title and type name

GuilhermeSaraiva96 · 2025-02-19T14:10:45Z

file-formats/edoi/type/zif_aff_edoi_v1.intf.json

+{
+  "formatVersion": "1",
+  "header": {
+    "description": "Create eDocument Types",


does the I in EDOI stand for Interface? Then maybe the description could be: AFF for eDocument Interface (EDOI)

Add files for EDOI & EDOT

a8ec8b7

i506210 added 3 commits January 27, 2025 15:11

React to abaplint

499d83a

React to abaplint

e1e96b2

Merge branch 'main' into feature/edoi

ecdefa0

i506210 changed the title ~~[EDOI][EDOT] Add new object types EDOI and EDOT~~ [EDOI] Add new object types EDOI Jan 28, 2025

React to comments from Michael Schneider

3a2e9b9

Markus1812 reviewed Jan 28, 2025

View reviewed changes

i506210 added 2 commits February 4, 2025 10:54

reaction to comments

6b8c5b2

Merge branch 'main' into feature/edoi

a158a2a

GuilhermeSaraiva96 requested changes Feb 17, 2025

View reviewed changes

corrections

2759148

i506210 requested a review from GuilhermeSaraiva96 February 19, 2025 13:14

GuilhermeSaraiva96 requested changes Feb 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EDOI] Add new object types EDOI #681

[EDOI] Add new object types EDOI #681

i506210 commented Jan 27, 2025

cla-assistant bot commented Jan 27, 2025 •

edited

Loading

cla-assistant bot commented Jan 27, 2025

schneidermic0 commented Jan 28, 2025

i506210 commented Jan 28, 2025

Markus1812 left a comment

Markus1812 Jan 28, 2025

i506210 Jan 28, 2025

schneidermic0 Jan 30, 2025

i506210 Feb 5, 2025

i506210 commented Jan 28, 2025

GuilhermeSaraiva96 Feb 19, 2025

GuilhermeSaraiva96 Feb 19, 2025

GuilhermeSaraiva96 Feb 19, 2025

GuilhermeSaraiva96 Feb 19, 2025

GuilhermeSaraiva96 Feb 19, 2025

GuilhermeSaraiva96 Feb 19, 2025

GuilhermeSaraiva96 Feb 19, 2025

	"! Interface dersion description
	"! Interface version description

	BEGIN OF ty_edoc_int_drc_attr,
	BEGIN OF ty_determination_attribute,

	TYPES ty_edoc_int_drc_attributes TYPE SORTED TABLE OF ty_edoc_int_drc_attr WITH UNIQUE KEY process_type.
	TYPES ty_determination_attribute TYPE SORTED TABLE OF ty_edoc_int_drc_attr WITH UNIQUE KEY process_type.

[EDOI] Add new object types EDOI #681

Are you sure you want to change the base?

[EDOI] Add new object types EDOI #681

Conversation

i506210 commented Jan 27, 2025

cla-assistant bot commented Jan 27, 2025 • edited Loading

cla-assistant bot commented Jan 27, 2025

schneidermic0 commented Jan 28, 2025

i506210 commented Jan 28, 2025

Markus1812 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i506210 commented Jan 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cla-assistant bot commented Jan 27, 2025 •

edited

Loading