New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ACA-3312] Add user-filter util class #6098

Merged

mauriziovitale merged 10 commits into develop from dev-iuliaib-ACA-3312

Sep 25, 2020

Contributor

iuliaib commented Sep 7, 2020

Please check if the PR fulfills these requirements

The commit message follows our guidelines

Tests for the changes have been added (for bug fixes / features)

Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

Bugfix

Feature

Code style update (formatting, local variables)

Refactoring (no functional changes, no api changes)

Build related changes

Documentation

Other... Please describe:

Add user-filter.util class that corresponds to user-filters : Retrieve and manage user filters section in APS1.X.
Add methods that check if a filter is not highlighted in order to avoid waiting seven seconds for isFilterHighlighted() to return false.

What is the current behaviour? (You can also link to an open issue here)

What is the new behaviour?

Does this PR introduce a breaking change? (check one with "x")

Yes

No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

iuliaib requested review from DenysVuika, eromano and popovicsandras as code owners

September 7, 2020 16:06

pr-triage bot added the PR: unreviewed label

codecov-commenter commented Sep 8, 2020 •

edited

Codecov Report

Merging #6098 into develop will increase coverage by 1.50%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #6098      +/-   ##
===========================================
+ Coverage    76.29%   77.80%   +1.50%     
===========================================
  Files          852      143     -709     
  Lines        19067     4005   -15062     
  Branches      3779      801    -2978     
===========================================
- Hits         14548     3116   -11432     
+ Misses        3378      630    -2748     
+ Partials      1141      259     -882

Impacted Files	Coverage Δ
lib/core/models/file.model.ts
lib/core/directives/logout.directive.ts
.../widgets/dynamic-table/editors/date/date.editor.ts
...ons/raphael-icon-google-drive-publish.component.ts
...-services/src/lib/task-list/models/filter.model.ts
lib/core/login/models/login-error.event.ts
...re/form/components/widgets/core/container.model.ts
...gram/components/diagram-sequence-flow.component.ts
lib/core/services/ecm-user.service.ts
...process-list/components/process-audit.directive.ts
... and 699 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b67b3b8...653b5b1. Read the comment docs.

iuliaib force-pushed the dev-iuliaib-ACA-3312 branch from 2f4034d to 9628215 Compare

September 14, 2020 06:41

popovicsandras requested changes

View reviewed changes

lib/testing/src/lib/process-services/actions/user-filters.util.ts Outdated

+                          return this.apiService.getInstance().activiti.userFiltersApi.createUserTaskFilter(new UserTaskFilterRepresentation(
+                              {appId: appId, name: newTaskFilterName, icon: iconName, filter: {sort: sortType, state: stateType, assignment: assignmentType}}));
+                      } catch (error) {
+                          Logger.error('Create Task Filter - Service error, Response: ', JSON.parse(JSON.stringify(error)));

Contributor

popovicsandras Sep 14, 2020

In these cases, don't we want rethrow the error, beside displaying them? Like this, even if an error occurs, we just show a message, but the program's execution will continue. Is it exactly what we want here?

Contributor

DenysVuika Sep 14, 2020

Indeed, the catch block will never be executed in case of the errors in the async createUserTaskFilter. I suggest not using try/catch here at all, as it's a test and we will see the error + stack trace anyway

Contributor Author

iuliaib Sep 21, 2020

@popovicsandras, @DenysVuika thanks for the comments. Regarding the catch's logger.error... I was writing this in the same idea of what we already have (eg. lib/process-services/src/lib/process-list/services/process.service.ts) and I was keeping it like this because I was wondering that if not, we will stop the execution of the tests and the entire class will fail, right? I'm not sure if I'm actually right.
Currently, in a class we have multiple tests, most of them with different preconditions [this is available at least for all the tests from adw->Process Services extension]. If one precondition (required for just one test) will fail, currently the tests will keep running. From my point of view, we could keep using this approach until we will find a way to avoid making an entire class fail because of one precondition needed by a single test. 🤔

@DenysVuika I'm not sure if I've understood correctly the concern, but createUserTaskFilter is an API used directly from js-api package (node_modules/@alfresco/js-api/src/api/activiti-rest-api/api/userFilters.api.ts). If something happens there, shouldn't be caught in async createATaskFilter catch's Logger.error? 🤔

lib/testing/src/lib/process-services/actions/user-filters.util.ts Outdated

+                      try {
+                          return this.apiService.getInstance().activiti.userFiltersApi.orderUserTaskFilters(new UserFilterOrderRepresentation({appId: appId, order: filtersIdOrder}));
+                      } catch (error) {
+                          Logger.error('Re-order the list of user task filters - Service error, Response: ', JSON.parse(JSON.stringify(error)));

Contributor

popovicsandras Sep 14, 2020

Same here.

lib/testing/src/lib/process-services/actions/user-filters.util.ts Outdated

+                      try {
+                          return this.apiService.getInstance().activiti.userFiltersApi.getUserTaskFilters({appId: appId});
+                      } catch (error) {
+                          Logger.error('List task filters - Service error, Response: ', JSON.parse(JSON.stringify(error)));

Contributor

popovicsandras Sep 14, 2020

Same here.

Contributor

mauriziovitale Sep 21, 2020

JSON.parse(JSON.stringify(obj)) is a hack to deep clone an object I don't see any reason why we should have it here

lib/testing/src/lib/process-services/actions/user-filters.util.ts Outdated

+                          });
+                          return chosenTaskFilter;
+                      } catch (error) {
+                          Logger.error('Get user task filters by name - Service error, Response: ', JSON.parse(JSON.stringify(error)));

Contributor

popovicsandras Sep 14, 2020

Same here.

lib/testing/src/lib/process-services/actions/user-filters.util.ts Outdated

+                      try {
+                          return this.apiService.getInstance().activiti.userFiltersApi.deleteUserTaskFilter(filterId);
+                      } catch (error) {
+                          Logger.error('Delete a task filter - Service error, Response: ', JSON.parse(JSON.stringify(error)));

Contributor

popovicsandras Sep 14, 2020

Same here.

lib/testing/src/lib/process-services/actions/user-filters.util.ts Outdated

+                          return this.apiService.getInstance().activiti.userFiltersApi.updateUserTaskFilter(filterId, new UserTaskFilterRepresentation(
+                              {appId: appId, name: updatedTaskFilterName, icon: updatedIconName, filter: {sort: updatedSortType, state: updatedStateType, assignment: updatedAssignmentType}}));
+                      } catch (error) {
+                          Logger.error('Delete a task filter - Service error, Response: ', JSON.parse(JSON.stringify(error))); }

Contributor

popovicsandras Sep 14, 2020

Same here.

pr-triage bot added PR: reviewed-changes-requested and removed PR: unreviewed labels

iuliaib force-pushed the dev-iuliaib-ACA-3312 branch from 9628215 to 7ad4a11 Compare

September 21, 2020 16:00

pr-triage bot added PR: unreviewed and removed PR: reviewed-changes-requested labels

iuliaib requested a review from popovicsandras

September 21, 2020 16:39

mauriziovitale requested changes

View reviewed changes

Contributor

mauriziovitale left a comment

check the comments

pr-triage bot added PR: reviewed-changes-requested PR: unreviewed and removed PR: unreviewed PR: reviewed-changes-requested labels

iuliaib added 9 commits

September 23, 2020 18:16


          Add user-filter api


          typo

acd787e


          Add user-filters API

856c1c7


          Add isFilterNotHighlighted method


          Add isProcessFilterNotHighlighted methods

eb537a7


          Add process-list constructor

c19458d


          Add isTasksFilterNotHighlighted methods and methods for custom task f…

c4fd4a1

…ilter


          Add getDataTable for process-

4e54c85

list


          Simplify the Logger.error

9e3e620

iuliaib force-pushed the dev-iuliaib-ACA-3312 branch from b6ffccd to 9e3e620 Compare

September 23, 2020 15:17

iuliaib requested a review from mauriziovitale

September 23, 2020 19:13

mauriziovitale reviewed

View reviewed changes

lib/testing/src/lib/process-services/actions/user-filters.util.ts Outdated Show resolved Hide resolved


          typo

653b5b1

mauriziovitale approved these changes

View reviewed changes

mauriziovitale merged commit df2e531 into develop

mauriziovitale deleted the dev-iuliaib-ACA-3312 branch

September 25, 2020 08:30

pr-triage bot added PR: merged and removed PR: unreviewed labels

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

Reviewers

mauriziovitale mauriziovitale approved these changes

DenysVuika Awaiting requested review from DenysVuika DenysVuika is a code owner

eromano Awaiting requested review from eromano eromano is a code owner

popovicsandras Awaiting requested review from popovicsandras popovicsandras is a code owner