Skip to content
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.

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

[ACS-3887] Rule set listing, including linked & inherited rules #2780

Conversation

TomikaArome
Copy link
Contributor

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:

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

https://alfresco.atlassian.net/browse/ACS-3887

What is the new behaviour?

All rule sets for a folder are listed.

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:

@TomikaArome TomikaArome changed the title Feature/dev thunter acs 3887 linked inherited rule sets listing [ACS-3887] Rule set listing, including linked & inherited rules Nov 16, 2022
FolderRuleSetsService,
FolderRulesService,
{ provide: Store, useValue: { dispatch: () => {} } },
// { provide: FolderRulesService, useValue: folderRulesServiceSpy },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unremoved commented code

let contentApiService: ContentApiService;

let callApiSpy: jasmine.Spy;
// let loadRulesSpy: jasmine.Spy;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here for comment

if (nodeId) {
return this.contentApi.getNode(nodeId).pipe(
catchError((error) => {
if (error.status === 404) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have some file with constants that we use across whole app? If not it may be worth to consider creating one for example for storing HTTP status codes as constants instead of using just raw numbers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, no, but HTTP codes are pretty well known so perhaps it's not as crucial here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah don't worry this is just an idea for the future

@TomikaArome TomikaArome force-pushed the feature/dev-thunter-ACS-3887-linked-inherited-rule-sets-listing branch from 77fed3d to 640d03f Compare November 16, 2022 15:02
Copy link
Contributor

@nikita-web-ua nikita-web-ua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!

@TomikaArome TomikaArome merged commit c75091b into develop Nov 16, 2022
@TomikaArome TomikaArome deleted the feature/dev-thunter-ACS-3887-linked-inherited-rule-sets-listing branch November 16, 2022 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants