adding extra services functionality

[eik-dahms]

feature: users can add non dataplant services
reason: local run gitlab/ datahub instances, testing and development of instances
resolves #56

Workflow:

New Service View

An additional panel has been added where additional services are show and can be managed.

Add Service Dialog

The user can add services with or without client secret. (Depends how application is set up in gitlab)

Remove Service Dialog

User can also remove previously added service.

---

This features strictly separates Dataplant services and services added by the user. Meaning the user can not change the hardcoded dataplant services or remove them. Added services are stored in a separate config file. Services with already existing URLs can not be added or changed.

---

In general UI, datamodels and functionality have been altered such that no functionality was taken and everything else works as expected.

[JonasLukasczyk]

Thank you for putting this together. This will add quite a lot of code, so I want to make sure that there is no simpler way to do this. For example, is there a good reason why you added an additional datahub file instead of just updating the original one? I think you could apply all your changes to this single file instead. Making the distinction between the original and additional case causes a lot of code bloat like:

export const CredentialStore: CredentialStoreType = {
  /** manage credentials for different datahubs */
  credential_file_dataplant: CREDENTIALS_DATAPLANT, 
  credential_file_additional: CREDENTIALS_ADDITIONAL, 
  credentials: {dataplant: {}, additional: {}},

[eik-dahms]

one reason for this, as pointed out before is to separate dataplant and user settings. such it is secured that users will have access to dataplant services and that they can not be interfered with and are updated properly, should it be required.

The code could be more elegant and efficient, I agree, but at the same time i did not want to change to much in the application code.

The main problem I see is how the configuration is handled at the moment.

First of all there are two different files for configuration. The main config and the configuration for the datahub services. So it is not quite clear where to put it? I did not want to put it in the datahub file because of what I mentioned before but it also did not make to much sense to put it in the main config.

Another reason is that it is unclear how config files will be updated and how and if user data is inteded to be conserved. The way it is setup right now - once arcitect is installed the config files will never be updated. Because the way "initConfig" in "init.ts" is designed. This function only adds configs if a config file is not existent. Which means once a user has installed ARCitect and they do not delete corresponding files or the complete arcitect directory no config files will be updated.
Furthermore I assume that at the moment you would replace complete files when changing the config? That would mean if I store the additional services in the datahub config and it gets replaced by a new datahub config file the user settings will be lost.

I think this mechanism could benefit from a more concise config definition and a refined update mechanism. For e.g.

{
  "application": {
     "description": "settings for ARCitect"
     "gitDebug": false,
     "toolbarMinimized": false,
     "showHelp": false,
     "showTooltips": false,
     "swate_url": "https://swate-alpha.nfdi4plants.org"
  },
  "services": {
       "description": "dataplant and additional services"
       "dataplant": [
         {},
         {}
        ],
      "additionalServices": [
         {},
         {}
      ]
}
}

This is of course just an example and the design should be discussed.
When updating the config, instead of replacing the complete file, I would only update values which can not be set by the user or config settings which have been added or removed. This could be done by hand, with the spread operator or by adding an additional property(flag) like setByUser, which would not change/update values in the underlying object unless it is necessary for functional purpose. (In that case there should be a mechanism to keep old user data if possible)

I would also consider having a config Object/ Class instead of reading/ writing to files. Such that the config is read once and the config object serves config values in backend/frontend and stores updated values to the underlying config file instead of saving the config directly everytime these values change.

Such design would make it easier in a way that one would simply get "services" from the config, which already contains dataplant and additional services and would lead to a smaller footprint in as the above mentioned code. At the same time I think consistency on App updates should be ensured.

Please let me know if you want to discuss this in more detail.

[JonasLukasczyk]

Ok, lets go through this one by one

one reason for this, as pointed out before is to separate dataplant and user settings. such it is secured that users will have access to dataplant services and that they can not be interfered with and are updated properly, should it be required.

The core idea here was to provide a default config that can be modified by the users, but they can always reset the config with the reset button. I can see that we could implement some additional mechanics to flag user changes and try to do an intelligent reset/merge, but I think this is really not necessary. Especially we thought about this and i can always produce counter examples where one time we want to override user settings and sometimes preserve them. So since this can not be handled in a consistent manner we think it makes sense to let the users make any changes they want, but when they hit the reset button they might need to add these changes again. So the goal is to make it as easy as possible for the users to make these changes (so that they can be repeated if necessary).

The code could be more elegant and efficient, I agree, but at the same time i did not want to change to much in the application code.

I think the current version adds much more changes.

The main problem I see is how the configuration is handled at the moment.

First of all there are two different files for configuration. The main config and the configuration for the datahub services. So it is not quite clear where to put it? I did not want to put it in the datahub file because of what I mentioned before but it also did not make to much sense to put it in the main config.

Settings of the ARCitect interface such as the visibility of the sidebar are handled in the ARCitect.json file, whereas the list of DataHubs and their credentials are stored in the DataHubs.json file. So far if expert users wanted to add a DataHub they had to manually edit the DataHub.json file. Making this possible through a dialog makes perfect sense and I also have no problems in merging these into one config file.

Another reason is that it is unclear how config files will be updated and how and if user data is inteded to be conserved. The way it is setup right now - once arcitect is installed the config files will never be updated. Because the way "initConfig" in "init.ts" is designed. This function only adds configs if a config file is not existent. Which means once a user has installed ARCitect and they do not delete corresponding files or the complete arcitect directory no config files will be updated.

There is the reset button.

Furthermore I assume that at the moment you would replace complete files when changing the config? That would mean if I store the additional services in the datahub config and it gets replaced by a new datahub config file the user settings will be lost.

Yes, and as described earlier I think this is a good approach because intelligent preservations are not possible anyway and cause a lot of additional code bloat than this simple approach which will work for most users.

I think this mechanism could benefit from a more concise config definition and a refined update mechanism. For e.g.

{
  "application": {
     "description": "settings for ARCitect"
     "gitDebug": false,
     "toolbarMinimized": false,
     "showHelp": false,
     "showTooltips": false,
     "swate_url": "https://swate-alpha.nfdi4plants.org"
  },
  "services": {
       "description": "dataplant and additional services"
       "dataplant": [
         {},
         {}
        ],
      "additionalServices": [
         {},
         {}
      ]
}
}

This is of course just an example and the design should be discussed. When updating the config, instead of replacing the complete file, I would only update values which can not be set by the user or config settings which have been added or removed. This could be done by hand, with the spread operator or by adding an additional property(flag) like setByUser, which would not change/update values in the underlying object unless it is necessary for functional purpose. (In that case there should be a mechanism to keep old user data if possible)

I would also consider having a config Object/ Class instead of reading/ writing to files. Such that the config is read once and the config object serves config values in backend/frontend and stores updated values to the underlying config file instead of saving the config directly everytime these values change.

Such design would make it easier in a way that one would simply get "services" from the config, which already contains dataplant and additional services and would lead to a smaller footprint in as the above mentioned code. At the same time I think consistency on App updates should be ensured.

Please let me know if you want to discuss this in more detail.

So overall I would suggest the following approach

1. right now the initial configurations exist as resource files. I would move them to the code and merge both the application settings and datahub credentials into one JSON object.
2. When the user makes changes then only the part of the JSON object that differs from the default are written to disk.
3. A reset button deletes local settings and thus resets arcitect to default settings.
4. A dialog in the Settings View enables users to easily add/edit datahub credentials.

I think this will not result in a lot of new code and manages the settings in a better way. If you are ok with this then I will implement these changes.

[eik-dahms]

sorry that should be merged v.0.0.54 into feature branch

[eik-dahms]

So overall I would suggest the following approach

1. right now the initial configurations exist as resource files. I would move them to the code and merge both the application settings and datahub credentials into one JSON object.
2. When the user makes changes then only the part of the JSON object that differs from the default are written to disk.
3. A reset button deletes local settings and thus resets arcitect to default settings.
4. A dialog in the Settings View enables users to easily add/edit datahub credentials.
   If you are ok with this then I will implement these changes.

I think this should work. you could also implement the config part and after that I can adapt my code to the new system.

I think this will not result in a lot of new code and manages the settings in a better way.

I am struggeling a bit trying to understand what the issue with the amount of code is? The amount added is not that much and at the same time a bigger amount of code has been deleted. Additionally some part of the new code is also types from Typescript - which will not be compiled into the final code.

In general the size of the application code is miniscule compared to code added by libraries and the electron app. The code changes are not visible in the final app size.

This is not a web application in the sense that scripts have to be loaded from a distant server, which requires small script sizes in order to load the page as fast as possible.

And finally, to my humble knowledge, more code does not imply worse code. It has to be considered in the context of what the added code actually solves and if abstractions and changes can be beneficial in the future. The size of the added code mostly results from workarounds for the current code structure, bug fixes (e.g. Service View will hang if request to server returns connection error, which is not processes or catched in the "getJson" function, which results in nothing because the function does not properly implement Promises) and furter I worked with a bit more abstraction then the current code. This is necessary, from my point of view, as this is an open source project and proper abstraction can be beneficial for new features and avoids hardcoding every solution - such that it can get realy tidious to change something in the code.

If you prefer less "bloaty" code in the future I would restrain from above mentioned processes in the future and would try to implement everything with as less code as possible.

[JonasLukasczyk]

Sorry for the confusion, I'm not concerned about performance, this is solely about maintainability. Since the config mechanism is a central aspect of the app I just wanted to make sure that we move forward with a sensible approach before we add too much code that essentially already restricts us on a specific solution. This is why I think it was very beneficial to have this discussion here. As soon as the new config system is in place we can adapt your changes.