No Master Domain in Admin UI

Taskana / taskana

Lightweight library for general purpose task management

https://taskana.pro

Apache License 2.0

96 stars 101 forks source link

No Master Domain in Admin UI #2201

Closed gitgoodjhe closed 1 year ago

gitgoodjhe commented 1 year ago

The Master Domain is not shown in the TASKANA Admin UI.

Before TASKANA 6.0 there was a workaround: Configuring an "empty" domain like this:

taskana.domains=DOMAIN1, ,DOMAIN2,DOMAIN3

This seems to not be possible anymore with the new | delimiters

arolfes commented 1 year ago

To be more precise:

the visibility of master domain is depend from the customer installation. Therefore it should be possible to switch it off.

The visibility of master domain should be different then the other "configured" domains

mustaphazorgati commented 1 year ago

Hi @gitgoodjhe!

We have already discussed that the master domain "" is an awful name (a long time ago). Because of that we have introduces SharedConstants#MASTER_DOMAIN. My suggestion: Fix this issue the proper way: Rename the master domain to e.g. MASTER_DOMAIN and provide database update scripts for this.

This forces us to do at least a minor release. I am aware that this is technically breaking, but this change is so small, that it could be considered a minor aswell

mustaphazorgati commented 1 year ago

Hi @gitgoodjhe!

We have already discussed that the master domain "" is an awful name (a long time ago). Because of that we have introduces SharedConstants#MASTER_DOMAIN. My suggestion: Fix this issue the proper way: Rename the master domain to e.g. MASTER_DOMAIN and provide database update scripts for this.

This forces us to do at least a minor release. I am aware that this is technically breaking, but this change is so small, that it could be considered a minor aswell

If we would consider that, this would be our TODOs:

Rename SharedConstants#MASTER_DOMAIN to a proper value
Adjust all our test sql data
Provide sql update script for each database
Adjust Frontend (administration-overview.component.html) and remove the previously existing edgecase
[optional] highlight the master domain somehow using a different css class. [design: TBD]

@gitgoodjhe what do you think?

arolfes commented 1 year ago

Hi @gitgoodjhe,

how urgent is this change/bugfix for you?

Which solution do you prefere? Ugly in the UI or properly in the backend with a release?

gitgoodjhe commented 1 year ago

Hi @arolfes let's talk about this in our upcoming sync call

arolfes commented 1 year ago

We decided to always add the master domain to the configuration as empty string at last position of configured domains.

therefore we switch from Set to List, because its sorted and add an empty string to last position of domains list

also we agreed to switch classificationTypes from Set to List