-
Notifications
You must be signed in to change notification settings - Fork 45
Add reference for deleted user #5494
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
Add reference for deleted user #5494
Conversation
|
found by @MSoeb |
MSoeb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works as intended
|
Waitng for the backend issue/ independent testing OpenSlides/openslides-backend#3164 |
luisa-beerboom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed some relatively minor stuff. If any of my remarks don't work or you don't see them as practival for whatever reason, feel free to tell me
client/src/app/site/base/base-filter.service/base-filter-list.service.ts
Outdated
Show resolved
Hide resolved
...eetings/pages/motions/pages/amendments/components/amendment-list/amendment-list.component.ts
Outdated
Show resolved
Hide resolved
...otion-detail/pages/motion-view/components/motion-detail-diff/motion-detail-diff.component.ts
Outdated
Show resolved
Hide resolved
| } else if (model.getTitle() === this.translate.instant(`Deleted user`)) { | ||
| this._removeUsersMap[model.user_id] = model.id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this even a possible case?
I don't see any new code causing model.getTitle() to display Deleted user (and translated no less), so would this even work?
Also I'd imagine if the getTitle says Deleted user, it must either be a person with a really funny name (who could be treated like any other user) or a motion meeting user link with no attached meeting user, which could therefore not have a user_id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this even a possible case?
Yes, if a user is deleted (and not removed)
The string is inserted at motion-manage-motion-meeting-users.component.html
Yes it's also translated
this line is only called if the user has no user_id, hence the user is not a "real" user
a motion meeting user link with no attached meeting user, which could therefore not have a user_id
I do not know how to rcreate such a case
I tested it with a user who is not deleted but removed and that user is displayed like before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I resolved it?
...ges/meetings/pages/motions/pages/motion-list/components/motion-list/motion-list.component.ts
Outdated
Show resolved
Hide resolved
...p/site/pages/meetings/pages/motions/services/export/motion-pdf.service/motion-pdf.service.ts
Outdated
Show resolved
Hide resolved
bspekker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an issue after creating a new user in one of the concerned fields: when saving the entry the edit doesn't close and an error will appear when trying to save again.
luisa-beerboom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor things
resolves #5324
needs [https://github.com/OpenSlides/openslides-autoupdate-service/pull/1274]
and [https://github.com/OpenSlides/openslides-backend/pull/3164]