Skip to content

[AUTH] Add new object type AUTH - #849

Open
WDFdaniel wants to merge 7 commits into
SAP:mainfrom
WDFdaniel:feature/auth-only
Open

[AUTH] Add new object type AUTH#849
WDFdaniel wants to merge 7 commits into
SAP:mainfrom
WDFdaniel:feature/auth-only

Conversation

@WDFdaniel

Copy link
Copy Markdown
Contributor

No description provided.

@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

👋 Hi — I'm PR Bot, your SAP code review assistant.

I'll automatically review your pull requests for code quality, security, and SAP compliance. Get an overview of what I do →

What I do

  • Summarize your pull request changes
  • Review code for quality, correctness, and reliability
  • Suggest fixes when a pipeline job fails

Key commands

Command Description
/review Trigger a code review
/summarize Generate a PR summary
/ask <question> Ask about the current changes
/help See all available commands
Configure me for your team

Create .hyperspace/pull_request_bot_config.json in your repository:

{
  "$schema": "https://devops-insights-pr-bot.cfapps.eu10-004.hana.ondemand.com/schema/pull_request_bot.json",
  "features": {
    "control_panel": false,
    "summarize": {
      "auto_generate_summary": true,
      "auto_insert_summary": true,
      "auto_run_on_draft_pr": true,
      "use_custom_summarize_prompt": false,
      "use_custom_summarize_output_template": false,
      "excluded_paths": [],
      "auto_exclude_authors": []
    },
    "review": {
      "auto_generate_review": true,
      "auto_run_on_draft_pr": false,
      "use_custom_review_focus": false,
      "excluded_paths": [],
      "auto_exclude_authors": []
    },
    "sonar_fix": {
      "enable": true,
      "excluded_rules": []
    },
    "pipeline_fix": {
      "enable": true
    }
  },
  "excluded_paths": []
}

Full configuration reference →

@hyperspace-pr-bot

Copy link
Copy Markdown
Contributor

Control Panel

Hi, I'm an AI-powered Review Bot that helps you with summarizing and reviewing pull requests.
To interact with me, just use the following actions:

  • 📝 Summarize PR
  • 🔍 Review
  • 🗑️ Delete all bot comments and reviews

@hyperspace-pr-bot hyperspace-pr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR introduces the AUTH (Authorization Field) object type with several issues that need to be addressed before merging: the wrong header type is used (ty_header_60_src instead of ty_header_60 for a non-source DDIC object), both enum fields with no initial-value constant are missing $required, the README's file structure table is missing the example column link, and all four example JSON files are missing a trailing newline as required by the repository's .editorconfig.

PR Bot Information

Version: 1.29.33

"! <p class="shorttext">Header</p>
"! Header
"! $required
header TYPE zif_aff_types_v1=>ty_header_60_src,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong Header Type: AUTH (Authorization Field) is not a source-code object, so ty_header_60_src is incorrect here.

ty_header_60_src is for source-code objects (CLAS, INTF, FUGR, PROG, etc.) and exposes the abapLanguageVersion enum with standard, keyUser, and cloudDevelopment values. Authorization Fields are DDIC non-source objects and should use ty_header_60 (supports standard + keyUser + cloud) or ty_header_60_no_abap_lv if the object has no ABAP language version concept at all.

The generated schema at lines 32–52 of auth-v1.json already shows abapLanguageVersion being emitted with standard/keyUser/cloudDevelopment enum values, which is the non-source set — so ty_header_60 is the correct choice. Consider replacing ty_header_60_src with ty_header_60.

Suggested change
header TYPE zif_aff_types_v1=>ty_header_60_src,
header TYPE zif_aff_types_v1=>ty_header_60,

Double-check suggestion before committing. Edit this comment for amendments.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

Comment thread file-formats/auth/type/zif_aff_auth_v1.intf.abap
Comment thread file-formats/auth/type/zif_aff_auth_v1.intf.abap
Comment thread file-formats/auth/examples/z_aff_example_auth_1.auth.json
Comment thread file-formats/auth/README.md Outdated

File | Cardinality | Definition | Schema | Example
:--- | :--- | :--- | :--- | :---
`<name>.auth.json` | 1 | [`zif_aff_auth_v1.intf.abap`](./type/zif_aff_auth_v1.intf.abap) | [`auth-v1.json`](./auth-v1.json)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README file structure table is missing the Example column link: The File Structure table header declares five columns (File | Cardinality | Definition | Schema | Example) but the data row only has four cells — there is no example file link in the last column. Every other README in the repo (e.g., clas/README.md) provides an example link. Please add the example file reference, e.g. [z_aff_example_auth_1.auth.json](./examples/z_aff_example_auth_1.auth.json).

Suggested change
`<name>.auth.json` | 1 | [`zif_aff_auth_v1.intf.abap`](./type/zif_aff_auth_v1.intf.abap) | [`auth-v1.json`](./auth-v1.json)
`<name>.auth.json` | 1 | [`zif_aff_auth_v1.intf.abap`](./type/zif_aff_auth_v1.intf.abap) | [`auth-v1.json`](./auth-v1.json) | [`z_aff_example_auth_1.auth.json`](./examples/z_aff_example_auth_1.auth.json)

Double-check suggestion before committing. Edit this comment for amendments.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

@WDFdaniel

Copy link
Copy Markdown
Contributor Author

The structure and information in AFF are mainly based on the AUTH ADT editor.

@GuilhermeSaraiva96

Copy link
Copy Markdown
Contributor

At first sight, this AFF seems to not have any overlap/common fields with SUSI/SUSH and TRAN. Is that correct?

@WDFdaniel

Copy link
Copy Markdown
Contributor Author

At first sight, this AFF seems to not have any overlap/common fields with SUSI/SUSH and TRAN. Is that correct?

Yes, that's correct.

@GuilhermeSaraiva96 GuilhermeSaraiva96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general it looks pretty good!

data_element TYPE c LENGTH 30,
"! <p class="shorttext">Organizational Level</p>
"! Organizational level information
org_level_info TYPE c LENGTH 120,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are the possibles values for the organisational level? any description? I could only find in existing objects: Field is not defined as Organizational level.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the text ‘Field cannot be defined as organizational level’ is possible (e.g. for auth field ACTVT) or ‘Field is defined as Organizational level with Variable $PLVAR.’ (e.g. for auth field PLVAR). The last text contains a variable part, in this example ‘$PLVAR’. This field will be read-only via configuration, but I can enhance the description.

"! <p class="shorttext">Exit Function Module</p>
"! The authorization field uses an individual maintenance dialog that is implemented in the named function module
"! and overrides the standard maintenance dialog.
exit_function_module TYPE c LENGTH 30,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could not find this field in ADT. How is this currently solved in ADT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently this information is concatenated to the text of one radio button in the section 'Maintenance Dialog', like “Authorization Maintenance using Exit Module F4_PFCG_SICF” for authorization field ICF_NODE. This field in AFF will be set to read only via configuration.

Comment thread file-formats/auth/.DS_Store Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"! <p class="shorttext">Search Help in Standard Maintenance Dialog</p>
"! Search help in standard maintenance dialog
"! $required
provide_search_help TYPE ty_search_help,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
provide_search_help TYPE ty_search_help,
search_help TYPE ty_search_help,

I think the verb 'provide' could be misleading as this field represents the configuration for the search help

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"exitFunctionModule": "F4_PFCG_SICF"
},
"provideSearchHelp": {
"checkTable": "",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkTable is not required and is empty -> delete field

Suggested change
"checkTable": "",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

},
"maintenanceDialog": {
"typeOfMaintenanceDialog": "standard",
"exitFunctionModule": "",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"exitFunctionModule": "",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

},
"maintenanceDialog": {
"typeOfMaintenanceDialog": "standard",
"exitFunctionModule": ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"exitFunctionModule": ""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"exitFunctionModule": ""
},
"provideSearchHelp": {
"checkTable": "",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"checkTable": "",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

},
"maintenanceDialog": {
"typeOfMaintenanceDialog": "standard",
"exitFunctionModule": ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"exitFunctionModule": ""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread file-formats/auth/README.md Outdated

File | Cardinality | Definition | Schema | Example
:--- | :--- | :--- | :--- | :---
`<name>.auth.json` | 1 | [`zif_aff_auth_v1.intf.abap`](./type/zif_aff_auth_v1.intf.abap) | [`auth-v1.json`](./auth-v1.json) | [`z_aff_example_auth_1.auth.json`](./examples/z_aff_example_auth_1.auth.json)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README only links example 1, but 4 examples were added. Link all of them in the Example column using short labels to keep the table compact:
1, 2,
3, 4

(See the markdown code, as these were rendered to a link)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants