Opened 2 years ago
Closed 2 years ago
#1369 closed defect (fixed)
Search shows only one out of two users
Reported by: | hoffmann | Owned by: | arescope |
---|---|---|---|
Priority: | high | Milestone: | v1.1 |
Component: | Search | Version: | 0.99 |
Keywords: | Cc: |
Description
When two users have identical e-mail addresses (maybe it is necessary that they have also identical last/first name, I did not check), then only one of them is displayed when searching on
https://www.cta-observatory.org/indico/userList.py?sSurName=cheikh
(or equivalent) and other search dialogs in InDiCo?.
That should (and hopefuly can) be changed rapidly.
While it is correct that new users cannot "reuse" an existing e-mail address, they may change their address afterwards. Thus it may happen that inactive accounts are displayed, while active accounts are "hidden" by the system. A merge (over HTTP) needs a working search, so it is not possible any more. (Well, a workaround is to change the address slightly for one record, and both appear again.)
Merge with indico_shell also works fine. But better fix the problem in the search result display.
Change History (18)
comment:1 Changed 2 years ago by hoffmann
comment:2 Changed 2 years ago by jbenito
- Milestone changed from v1.1 to v1.2
comment:3 follow-up: ↓ 4 Changed 2 years ago by jbenito
- Milestone changed from v1.2 to v1.1
- Owner set to arescope
- Status changed from new to assigned
comment:4 in reply to: ↑ 3 Changed 2 years ago by hoffmann
Replying to jbenito:
Oh, if it is back on v1.1 milestone, I have to give my comments quicker ;-)
- I beleive I understand a bit better the implications of search now: The one in "userList.py" does not seem to yield exactly the same as the one in the AJAX window/widget, which is used for example to fill the "user merge" users or to find users in almost every place elsewhere in InDiCo.
(By the way: The .../userList.py?sSurname=hoff way to search users from InDiCo starting from external URLs is extremely helpful. Do never remove it!)
- There are three states in the life of an avatar: unconfirmed, active, deleted. My original report concerned only the first to states. However, in some cases it can be useful to target a search on status (active being the default, active+unconfirmed seeming to be the presently implemented filter). One or several checkboxes (alike "include disabled accounts" for example) would be useful for systems, where the standard InDiCo user registry is still used.
So, while you are on it, maybe you want to take into account these points, too.
comment:5 follow-up: ↓ 6 Changed 2 years ago by arescope
Hi,
After taking a look I discover why only showing only one record. It is because we use the email as key in multiples places. It is because we take into account that the email can be only registered one, and shall be unique. The problem related to might be due a previous bug where the email could be changed. Now you cannot create new users with an existing email, event if its unconfirmed, and you cannot either change to an existing used email.
So I suggest you to modify manually the email of the duplicated with:
av = AvatarHolder().getById(USERID) av.setEmail(NEWEMAIL) dbi.commit()
Anyway we are going to write in the log a Warning message about the issue in order to let the administrators handle the problem.
Cheers,
Alberto
comment:6 in reply to: ↑ 5 ; follow-up: ↓ 7 Changed 2 years ago by hoffmann
Replying to arescope:
After taking a look I discover why only showing only one record. It is because we use the email as key in multiples places. It is because we take into account that the email can be only registered one, and shall be unique. The problem related to might be due a previous bug where the email could be changed. Now you cannot create new users with an existing email, event if its unconfirmed, and you cannot either change to an existing used email.
You are right. This bug implies another ticket: Emails can be changed to produce duplicates (as opposed to your statement!).
Unless something changed wrt 0.99, try (on InDiCo? built-in registry):
- Create user A, email a@…
- Create user B, email b@…
- (Approve (validate) both!)
- Edit user A's email to be b@….
Now user A and B have email b@… :-)
So I suggest you to modify manually the email of the duplicated with:
av = AvatarHolder().getById(USERID) av.setEmail(NEWEMAIL) dbi.commit()
Anyway we are going to write in the log a Warning message about the issue in order to let the administrators handle the problem.
No worry, I found a workaround by editing the duplicate e-mail addresses. (Still, in your recipe I would need to know the USERID of the "hidden" user.)
However, as long as the database itself does not contain a constraint about duplicate e-mails, you canot assume that there are not any! Therefore the search should take it into account.
I am not sure, if a log message is sufficient. It should be a periodic check to be really efficient. A single message in a long log may be drowned in the administrator's inbox.
And in any case, please note: My remark about waiting, approved and deleted avatar status is still valid. (Sorry, this ticket has several facets, but I am also just discovering.)
comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8 Changed 2 years ago by arescope
Replying to hoffmann:
Replying to arescope:
After taking a look I discover why only showing only one record. It is because we use the email as key in multiples places. It is because we take into account that the email can be only registered one, and shall be unique. The problem related to might be due a previous bug where the email could be changed. Now you cannot create new users with an existing email, event if its unconfirmed, and you cannot either change to an existing used email.
You are right. This bug implies another ticket: Emails can be changed to produce duplicates (as opposed to your statement!).
Unless something changed wrt 0.99, try (on InDiCo? built-in registry):
- Create user A, email a@…
- Create user B, email b@…
- (Approve (validate) both!)
- Edit user A's email to be b@….
Now user A and B have email b@… :-)
I tried in v1.1 and that is not possible in v1.1 :-). So it cannot be either created or modified to an existing email.
So I suggest you to modify manually the email of the duplicated with:
av = AvatarHolder().getById(USERID) av.setEmail(NEWEMAIL) dbi.commit()
Anyway we are going to write in the log a Warning message about the issue in order to let the administrators handle the problem.
No worry, I found a workaround by editing the duplicate e-mail addresses. (Still, in your recipe I would need to know the USERID of the "hidden" user.)
However, as long as the database itself does not contain a constraint about duplicate e-mails, you canot assume that there are not any! Therefore the search should take it into account.
I am not sure, if a log message is sufficient. It should be a periodic check to be really efficient. A single message in a long log may be drowned in the administrator's inbox.
And in any case, please note: My remark about waiting, approved and deleted avatar status is still valid. (Sorry, this ticket has several facets, but I am also just discovering.)
I understand the point that we cannot put the constraint in the database but we already make the constraint in code now.
We could make an script but we think it is not necessary because it was a bug, and now its solved in the v1.1 so that issue will not happen again.
We think with that log message and update to v1.1 we do not need any check on top.
Best regards,
Alberto
comment:8 in reply to: ↑ 7 Changed 2 years ago by hoffmann
Replying to arescope:
I understand the point that we cannot put the constraint in the database but we already make the constraint in code now.
We could make an script but we think it is not necessary because it was a bug, and now its solved in the v1.1 so that issue will not happen again.
We think with that log message and update to v1.1 we do not need any check on top.
I agree. That's a way to solve the problem.
Remains the question how to find "deactivated" accounts (see above) with search.
comment:9 Changed 2 years ago by arescope
Hi,
I made an small patch with a checkbox in order to show only active users, that it will be unchecked by default.
The default behaviour, that is the one currently in v1.1 is that it will show not confirmed, active and disabled.
Best regards,
Alberto
comment:10 Changed 2 years ago by arescope
- Status changed from assigned to awaiting_merge
comment:11 Changed 2 years ago by hoffmann
OK, I do not know in which dialog you made the checkbox. But in some dialogs/circumstances it may be important not to allow inactive users (adding to groups for example).
comment:12 follow-up: ↓ 13 Changed 2 years ago by arescope
Hi,
I did in the userList.py search functionallity.
for the popup dialogs always search active users only, because the popup search is for users where you want to give permissions, etc so need to be active.
comment:13 in reply to: ↑ 12 ; follow-up: ↓ 14 Changed 2 years ago by hoffmann
Replying to arescope:
for the popup dialogs always search active users only, because the popup search is for users where you want to give permissions, etc so need to be active.
No, in the userMerge.py I want to be able to merge active and inactive users!
comment:14 in reply to: ↑ 13 ; follow-up: ↓ 15 Changed 2 years ago by jbenito
for the popup dialogs always search active users only, because the popup search is for users where you want to give permissions, etc so need to be active.
No, in the userMerge.py I want to be able to merge active and inactive users!
Dirk, you can always activate the user before merging it. I believe Alberto has already done part of the development that you need but unfortunately we cannot continue working on this for so long (merging active and inactive users requires a bit more of care). I would suggest that we can open a different ticket with this functionality for whoever can contribute (since, as you may understand, CERN is not benefiting from it. We do not have such a use case.).
comment:15 in reply to: ↑ 14 Changed 2 years ago by hoffmann
Replying to jbenito:
Dirk, you can always activate the user before merging it.
No, only when there is still an Identity left for the Avatar (which is not the case for our database).
But I agree to set this aspect to a lower priority.
comment:16 Changed 2 years ago by jbenito
Ok, ticket created #1376
comment:17 Changed 2 years ago by jbenito
- Status changed from awaiting_merge to merging
comment:18 Changed 2 years ago by jbenito
- Resolution set to fixed
- Status changed from merging to closed
PS, just for the records: ah.mergeAvatar from indico_shell did not seem to do the job.