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

Create user service #217

Merged
merged 9 commits into from
Nov 2, 2020
Merged

Create user service #217

merged 9 commits into from
Nov 2, 2020

Conversation

tobiasschweizer
Copy link
Contributor

@tobiasschweizer tobiasschweizer commented Oct 30, 2020

resolves DSP-775

  • add user service that instantiates a UserCache
  • prevent calls for system user (standoff link created by system user)

To be done in another PR:

@tobiasschweizer tobiasschweizer self-assigned this Oct 30, 2020
@tobiasschweizer tobiasschweizer added bug Something isn't working enhancement New feature or request labels Oct 30, 2020
}
);
// prevent getting info about system user (standoff link values are managed by the system)
if (this.displayValue.attachedToUser !== 'http://www.knora.org/ontology/knora-admin#SystemUser') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdelez This prevents requesting user info for the system user. Is this fine (the user prop) remains undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The info box does not work properly for the system user:

Screenshot 2020-11-02 at 14 28 12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 089c5af

@tobiasschweizer
Copy link
Contributor Author

tobiasschweizer commented Nov 2, 2020

@mdelez I figured that standoff links should be flagged as readonly since they cannot be edited anyway. They are managed by the system. DSP task: https://dasch.myjetbrains.com/youtrack/issue/DSP-974

Copy link
Contributor

@mdelez mdelez left a comment

Choose a reason for hiding this comment

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

Neat :)

@tobiasschweizer
Copy link
Contributor Author

@mdelez I think it would be a good idea to mention this service someplace so that it can also be used by other components (rather than requesting users from Knora directly). What would fit best?

@tobiasschweizer
Copy link
Contributor Author

I reapplied the optional chaining operator in 98d2c34

@tobiasschweizer
Copy link
Contributor Author

@mdelez I think the creator info (getTooltipText) could be generated when obtaining the user info.

When a method is called from a template like here matTooltip="{{getTooltipText()}}" it gets triggered many times. Instead, the method could be called when the user info is there, and its result could be saved to a class property which then could be used from the template. What do you think? Should we make a task for this?

@mdelez
Copy link
Contributor

mdelez commented Nov 2, 2020

@mdelez I think the creator info (getTooltipText) could be generated when obtaining the user info.

When a method is called from a template like here matTooltip="{{getTooltipText()}}" it gets triggered many times. Instead, the method could be called when the user info is there, and its result could be saved to a class property which then could be used from the template. What do you think? Should we make a task for this?

I've noticed this as well when calling methods from the template. I see it quite a lot on StackOverflow but I'm getting the impression that it shouldn't be done? I'm open to any suggestions that prevent multiple triggers for no real reason.

@tobiasschweizer
Copy link
Contributor Author

tobiasschweizer commented Nov 2, 2020

I've noticed this as well when calling methods from the template. I see it quite a lot on StackOverflow but I'm getting the impression that it shouldn't be done? I'm open to any suggestions that prevent multiple triggers for no real reason.

I think it is totally fine when reacting to an actual event like click etc.

In the case at hand, however, the tooltip message can be generated once and then be used in the template:

tooltipMsg = '';

onInit() {

this._userService.getUser.subscribe(
    user => this.getTooltipText()
);

}

getTooltipText(): string {
        const creationDate = 'Creation date: ' + this.displayValue.valueCreationDate;

        const creatorInfo = this.user ? '\n Value creator: ' + this.user?.givenName + ' ' + this.user?.familyName : '';

        this.tooltipMsg = creationDate + creatorInfo;
    }

and then in the template:

<button mat-button
                        class="info"
                        matTooltip="{{tooltipMsg}}"
                        matTooltipClass="info-tooltip">
                        <mat-icon>info</mat-icon>
                </button>

@tobiasschweizer tobiasschweizer merged commit 7be087a into main Nov 2, 2020
@tobiasschweizer tobiasschweizer deleted the wip/dsp-775-user-info branch November 2, 2020 15:52
RalfBarkow pushed a commit that referenced this pull request Nov 4, 2020
* main:
  Update dependencies (#219)
  Create user service (#217)
  DSP-759 Check if a Given Date Can Be Edited (#214)
  DSP-920 Renaming default github branch to "main" (#216)
  DSP-881 - Value Component Base Class Changes (#210)
  chore (package.json): update dsp-js-lib and adapt assertions (#215)
  DSP-522 Refactor search panel style (#206)
  Wip/dsp 885 default gravsearch query (#211)

# Conflicts:
#	Makefile

( DSP-987 ) Still a bug in expert search
( DSP-885 ) Expert search default/example Gravsearch query throws a 400 Bad Request error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants