feat: Add prompting code of Adaptation Project generator #2169
Conversation
🦋 Changeset detectedLatest commit: eb5f8a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…/open-ux-tools into feat/2162/move-adp-prompting
…/open-ux-tools into feat/2162/move-adp-prompting
…/open-ux-tools into feat/2162/move-adp-prompting
…/open-ux-tools into feat/2162/move-adp-prompting
| * | ||
| * @returns {boolean} - if provider is connected to ABAP system. | ||
| */ | ||
| public getIsConnected(): boolean { |
There was a problem hiding this comment.
why this wrapper? wouldn't be a getter/setter a nicer option if it is just about preventing it to be changed from the outside? If you really need this function then please rename it to isConnected and the variable to connected.
| if (manifest['sap.ovp']) { | ||
| return ApplicationType.FIORI_ELEMENTS_OVP; | ||
| } else if (hasSmartTemplateId || isSmartTemplate) { | ||
| return ApplicationType.FIORI_ELEMENTS; |
There was a problem hiding this comment.
You are not catching Fiori elements for OData v4 here.
There was a problem hiding this comment.
We don't need to differentiate between fiori elements v2 or v4, we don't have such logic which relies on that.
| * - If none of these conditions are met, the function defaults to categorizing the application as Free Style. | ||
| * - If the manifest is empty, it returns None. | ||
| */ | ||
| export function getApplicationType(manifest: Manifest): ApplicationType { |
There was a problem hiding this comment.
Why do we actually need the application type? I thought that was something we removed from the templates.
There was a problem hiding this comment.
We check the type because we need to see if the Adaptation Project supports it. These are supported:
FIORI_ELEMENTS = 'FioriElements',
FIORI_ELEMENTS_OVP = 'FioriElementsOVP',
FREE_STYLE = 'FreeStyle'
If it is something else we return an error stating that: ... Adaptation project doesn't support the selected application.
There was a problem hiding this comment.
You return one of these three if Object.keys(manifest).length > 0, so technically, you validations just checks if the manifest is neither undefined nor {}.
There was a problem hiding this comment.
We removed this check for a valid application type here 3bb3bc6#diff-034445208bf7ca40b734423c5cf343d6d0a4761a4b55cee844776b5458a1c9a0L86-L102.
But since this is used in the writing part we did not remove the method. It is used in a method that builds the descriptor content: createDescriptorChangeForResourceModel.
| */ | ||
| private constructor(private logger: ToolsLogger) { | ||
| this.endpoints = []; | ||
| this.isExtensionInstalled = isExtensionInstalledVsCode('sapse.sap-ux-application-modeler-extension'); |
There was a problem hiding this comment.
the extension sapse.sap-ux-application-modeler-extension is not a pre-requisite here. The extension allows modifying the stored systems, however, the systems itself are stored in the OS secure storage and can be added/manipulated with other tools as well, therefore, please remove that check.
There was a problem hiding this comment.
Then we need to check whether OS secure storage exists? Otherwise, if it doesn't, then how can we understand what prompts to show. If it exists then we need a list prompt with all the systems. If it doesn't exist then we show input prompts for system, client, username and password.
There was a problem hiding this comment.
Just call the store module and ask it for a list of systems. If at least one is returned you are good to go would be an option. The better option is that to reuse the functionality that is currently being implemented in the odata-service-inquirer because that one always offers a new system choice that then triggers the system creation prompts.
…/open-ux-tools into feat/2162/move-adp-prompting
…/open-ux-tools into feat/2162/move-adp-prompting
|
Feat for #2162.