Invert LoggingAspect blacklisting to whitelisting.

Currently the LoggingAspect works by blacklisting some methods. This has caused some trouble during TSK-1709. Should we change the blacklisting to a whitelisting, so that we don’t get confused when a given JointPoint gets weaved

Mustapha Zorgati

License header

Sebastian and I have discussed the automatic generation of license header together with our NT expert.
We have a lot of freedom and we’d love to discuss on which files to add license headers to.

Mustapha Zorgati

Extend ProcessEngineRequester for TaskanaAdapter

It would be nice to extend the ProcessEngineRequester, in such a way that we can retrieve more informations about tasks when getting tasks. For example the name of a task.
Importance can be understood when inspecting “process_with_different_variables_in_tasks_should_result_in_taskanaTasks_with_those_variables_in_custom_attributes"-method in TestTaskAcqusition. It’s not reproducible which task we get if we call the "getTaskIdsFromProcessInstanceId"-method in the ProcessEngineRequester.

Yakup Ensar Evli (Unlicensed)

Add a zip-method to our pro.taskana.common.internal.util.Pair class

The zip-method should combine each element of two List as Pair and return it as a List. Example:
List a = {1, 2, 3, 4}, List b = {5, 6, 7, 8} results in List c = {(1, 5), (2, 6), (3, 7), (4, 8)}.

Proposed piece of code: IntStream.range(0, a.size()).mapToObj(i -> Pair.of(a.get(i), b.get(i))).collect(Collectors.toList());

Yakup Ensar Evli (Unlicensed)

Unclear naming for data list in TestFactory methods

We are heavily using the @TestFactory annotation in order to create a test with multiple data sets. As of now there is no rule prohibiting the use of other mechanisms of the JUnit 5 arsenal to do the same thing.
* Should we enforce the usage of @TestFactory only for this usecase?
* Should we enforce the naming of the input list? (Now we have multiple variations: valuesForTests, no input list at all, testValues, testCases, setterList, iterator …)

Mustapha Zorgati

Rework Indices?

Currently our Indices are not complete and have to be reviewed again.
We see the following options:

• Rework Indices, remove non-basic ones

• integrate new basic indices

• Leave all Indices as is

  • Integrate any index currently used by our users

• Replace database handling by a framework (and thus the need of manually maintaining the indices)

Elena Mokeeva (Unlicensed) / Mustapha Zorgati

Rewrite ServiceLevelHandler

The logic in ServiceLevelHandler can be simplified or/and restructured. For example the usage of helper classes like DurationPrioHolder can be avoided.

04 Mar 2022 Wird nach dem 1.4. von Mustapha und Elena angegangen und ein konkreter Vorschlag unterbreitet.

Elena Mokeeva (Unlicensed)

Yakup Ensar Evli (Unlicensed)

• [L] Refactor TaskServiceImpl and ServiceLevelHandler (+ everything that is related)

Test for sorting according to two parameters

I couldn’t find test cases for sorting according to two parameters. We should test this.

Yakup Ensar Evli (Unlicensed)

Elena Mokeeva (Unlicensed)

• Create ticket

Don’t set workbasketSummary twice

In createTask method in TaskServiceImpl we set workbasketSummary twice in some cases.

Yakup Ensar Evli (Unlicensed)

Elena Mokeeva (Unlicensed)

• Create ticket

Split CompleteTaskAccTest

CompleteTaskAccTest contains tests for both, completion and claiming. Should we separate the claiming tests from the completion tests?

Elena Mokeeva (Unlicensed)

joerg.heffner

• Create ticket

SqlConnectionRunner Autocommit

Currently we have some logic in jobs like this:

Should we put this logic more top-level? For example directly in the SqlConnectionRunner?

joerg.heffner

Yakup Ensar Evli (Unlicensed) Elena Mokeeva (Unlicensed)

• Create ticket

TaskQuery start/end region Comments

What was the reason for introducing this? Do we want to keep it?

joerg.heffner

Holger Hagen (Unlicensed) Elena Mokeeva (Unlicensed)

Removed region comments

Rename GitHub repository + organization

we write TASKANA with caps. Our repository name is “taskana”, which technically is not ok. Furthermore our organization on GitHub is called “Taskana”, which is technically also not ok.

Should we rename our organization and the repository?

Mustapha Zorgati

Tristan Eisermann Yakup Ensar Evli (Unlicensed)

• [L] Discuss renaming taskana to TASKANA during Github migration

Undefined sorting order by Attachments

Currently the sorting order by Attachment is undefined. E.g. How to handle Tasks with multiple / no Attachments. In order to create a consistent API we have to discuss how we want to handle the sorting in those 2 cases.

Elena Mokeeva (Unlicensed)

• [L] In order to clear the intended functionality of sorting by Attachment, ask clients

Slack Huddles

Slack just released huddles (https://slack.com/intl/de-de/help/articles/4402059015315-Wir-stellen-vor--Slack-Team-Check-in-eine-neue-Art-Live-Audio-Besprechungen-durchzuf%C3%BChren )

I would love to use that feature to create a virtual office. That way we know who is currently active and can interact a little easier.
Let’s talk about that idea.

Mustapha Zorgati

Chi Nguyen (Unlicensed)

We don’t want to use Slack huddles (for now)

Why do we have a dedicated db2 SQL query

Currently the TaskQueryMapper has two implementations of queryTaskSummaries. Do we really need the dedicated implementation for db2

• [L] Execute a performace test to evaluate the need of the dedicated db2 query implementation

Rework version management

(Might be a misunderstanding from my side)

At this point, we have several versions of the some packages so it’s unclear which is used, because the version is not specified. The version of h2 was not specified, as well as hibernate.

It seems to be chaotic, maybe we can somehow structure it?

Elena Mokeeva (Unlicensed)

Yakup Ensar Evli (Unlicensed)

• Create ticket for Adapter to use spring-boot BOM

Decide on parameter names of intervals in TaskQuery, ClassificationQuery etc.

At the moment, some interval parameters are called just “intervals”, and others are called “modifiedWithin”, “createdWithin” etc. Which do we prefer?

Elena Mokeeva (Unlicensed)

Yakup Ensar Evli (Unlicensed)

• Ticket to refactor parameter names in *Query to include something time related to be created by Elena Mokeeva (Unlicensed)and include the topic below in the ticket

TSK-1952 - Getting issue details... STATUS

Decide on composed parameter names in Query entities

At the moment, some composed filter parameters are called with their full name (e. g. TaskQuery attachmentClassificationNameIn(String ... attachmentClassificationNames). Some other parameters have a short name, e. g. TaskQuery primaryObjectReferenceSystemInstanceNotLike(String... systemInstances);
Which do we prefer?

Elena Mokeeva (Unlicensed)

Mustapha Zorgati

Change as discussed on the fly as referenced above

BLOCKING RELEASE
Rename taskana-test-api module

We have a new module: taskana-test-api.

How should we name it? Holger Hagen (Unlicensed) said that that name could be a little confusing.

Mustapha Zorgati

• [L] Should we release the test-api?
• [L] Finalize test-api module naming

Add ‘skipAuthorization’ variable to TaskQuery

Currently, TaskQuery skips authorization checks if accesssIdIn=null. That might be confusing (There was a bug regarding this implementation). Proposal: add a ‘skipAuthorization’ variable.

Elena Mokeeva (Unlicensed)

Mustapha Zorgati Yakup Ensar Evli (Unlicensed)

postponed to TASKANA-201

• Wait for TASKANA-201

Refactor TestPriorityServiceProvider to be simpler

Currently the TestPriorityServiceProvider.java contains a lot of business logic which is totally irrelevant for TASKANA. Can we please discuss this and maybe reduce its complexity so that the tests are more readable?

Mustapha Zorgati

Yakup Ensar Evli (Unlicensed)

TestPriorityServiceProvider was deleted

JavaDoc: Referencing getter/setter with @see

our JavaDoc Rules state in 7. b) that we should link our getter and setters together with @see. Currently this is not done anywhere. Therefore we want to discuss this rule.

Elena Mokeeva (Unlicensed) / Mustapha Zorgati

Norman Schmidt

Rule was crossed out

JavaDoc: Enhancement for @return of getter methods

Currently the @return tag of our getter methods contains only trivial informations. Can we adjust the rules, so that we can get rid of those trivial informations?
Ideas:

• remove @return tag for getters

• use this pattern: @return <paramName>

• … (open for discussion)

Elena Mokeeva (Unlicensed) / Mustapha Zorgati

Yakup Ensar Evli (Unlicensed)

timebox 30-min and try to remove @return. If not possible use @return The <paramName>.

• Create ticket for @return JavaDoc changes.

Move Current Release Notes Page

When we create a release while some PRs are not merged yet, their current release notes have to be removed manually. This is an unnecessary step.
Suggestion: Replace the confluence page with a file inside our repository. After a release that file get’s “cleaned” automatically

Mustapha Zorgati

Sofie Hofmann (Unlicensed) Chi Nguyen (Unlicensed)

We have adjusted the PR template instead.

Make database limitations visible in Java API

We have database column restrictions for our entities. Should we make them visible in the java api? An example could be the usage of the javax validation API

Mustapha Zorgati

Yakup Ensar Evli (Unlicensed)

• Create ticket

Aria Label

Aria-label is an attribute on HTML elements to provide text alternatives for users who use assistive technology e.g. screen readers. Currently, it is only used in one component. Should we delete the attribute or add it wherever it is needed? I prefer to be consistent everywhere.

Sofie Hofmann (Unlicensed)

Chi Nguyen (Unlicensed) Tristan Eisermann

• [L] Talk about Aria Labels during the accessibility discussion

Implement conventional commits

Currently we don’t have a convention of how to write commit messages. We could adapt “Conventional commits” specification which is used widely in projects

https://www.conventionalcommits.org/en/v1.0.0/#summary

https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional

Chi Nguyen (Unlicensed)

Mustapha Zorgati Tristan Eisermann Martin Sawilla (Unlicensed)

• [L] introduce uniform commit messages (probably during the GitHub migration)

Naming of internal and api classes

Currently we name our variable inconsistently. Sometimes we name Impl classes with Impl and sometimes we don't.

Examples:

• taskanaEngineImpl vs taskanaEngine

• taskService vs taskServiceImpl

Should we unify this?

Mustapha Zorgati / Holger Hagen (Unlicensed)

Elena Mokeeva (Unlicensed)

• Yakup Ensar Evli (Unlicensed) Adjust guidelines:
  - variable name contains class name
  - short names are valid if unambiguos

Exception Signature of our Controllers

Currently our REST-Controllers list every Exception they can throw explicitly. Sometimes the list of Exceptions can be multiple lines. Should we throw Exception instead of listing every single Exception like we do with our Junit tests?

Mustapha Zorgati

Elena Mokeeva (Unlicensed)

• [L] discuss Exceptions in REST-Controllers later. If we decide to not change anything the second time, the discussion is closed

application-<database>.properties

Currently we have multiple application property files for each supported database.
Unfortunately only the main application property file (for H2) is maintained.

How do we want to ensure that the other property files are maintained?
I know that it is technically possible to use multiple property files in spring so that the database specific property files only contain the datasource.

A different approch is setting the datasource manually and start the database docker containers automatically.

Let’s talk about what’s reasonable and which usecases we have to consider

Mustapha Zorgati

Elena Mokeeva (Unlicensed) Yakup Ensar Evli (Unlicensed)

Postponed and to be explained by Mustapha Zorgati

• Discuss with Mustapha Zorgati
• Ticket to be created by Norman

Plural arguments in Queries

Currently, some arguments in Queries are in plural (ownerLongNames), and some in singular (attachmentClassificationId). Which one do we prefer?

Elena Mokeeva (Unlicensed)

Mustapha Zorgati Yakup Ensar Evli (Unlicensed) Elena Mokeeva (Unlicensed)

We agreed on using the plural form.

• Adjust Coding Guidelines
• Create ticket

‘descriptionIn’ filter option for ClassificationQuery is missing

We only have the option descriptionLike for filtering using the description of a Classification. Are other filter options needed?

Yakup Ensar Evli (Unlicensed) / Elena Mokeeva (Unlicensed)

we agreed not to do this now

Use a helper method for creating TaskHistoryEvent in TaskServiceImpl

At the moment, we have the following very similar code snippet in TaskServiceImpl several times.

Should we create a helper method that has the constructor of the TaskHistoryEvent as method parameter?

Elena Mokeeva (Unlicensed)

• [L] discuss adjustments of History SPI (wait for Mustapha to pitch his great idea about History SPI)
• create ticket (refactor of snippet) Norman Schmidt

Remove AbstractWorkbasketAccessItemQuery and AbstractWorkbasketAccessItemQueryImpl

Both classes are not used anywhere outside.

Yakup Ensar Evli (Unlicensed)

Mustapha Zorgati

• create ticket Norman Schmidt
• implement ticket Norman Schmidt

Extend HistoryEventManagerTest

We could check if there are untested scenarios involving HistoryEventManagerTest. They can be then added to HistoryEventManagerTest.

Elena Mokeeva (Unlicensed)

Coverage is better now

Getting rid of Thread.sleep

Sonarcloud complains about Thread.sleep (as Code smell). joerg.heffner proposed the idea of using Awaitlity (https://github.com/awaitility/awaitility ) for Taskana-Adapter to get rid of Thread.sleep. We should discuss if this solution is viable for Taskana too.

Yakup Ensar Evli (Unlicensed)

Elena Mokeeva (Unlicensed) Norman Schmidt

• Yakup Ensar Evli (Unlicensed) create tickets (taskana + adapter)
• Yakup Ensar Evli (Unlicensed) adapt guideline

if before debug-log

see comment in https://github.com/Taskana/taskana/pull/2003

Marcel Bagemihl

• create ticket Elena Mokeeva (Unlicensed)
• adapt logging concept Elena Mokeeva (Unlicensed)

Rewrite Test in TaskControllerIntTest

should_ThrowException_When_CreatingTaskWithInvalidParameter() doesn’t use RestHelper for communication. The method could probably be simplified by using RestHelper, like other methods in this test class already do.

Elena Mokeeva (Unlicensed)

• create ticket Norman Schmidt
• [L] examine JSON Blobs and how they can be removed (they can be built with RepresentationModelEntity instead of JSON Blob)