-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
ACLEditor: warn when adding ACL for unregistered user #7171
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -417,6 +417,24 @@ void ACLEditor::returnQuery(const MumbleProto::QueryUsers &mqu) { | |
| qhNameCache.remove(tid); | ||
| } | ||
| } | ||
| // Warn once per unregistered user that was just looked up. An unregistered | ||
| // user is one whose name was queried but for which the server did not return | ||
| // a valid registration ID in this response batch. | ||
| for (int i = 0; i < mqu.names_size(); ++i) { | ||
| const QString name = u8(mqu.names(i)); | ||
| const QString lname = name.toLower(); | ||
| if (qhNameWait.contains(lname) && !qsWarnedUnregistered.contains(lname)) { | ||
| qsWarnedUnregistered.insert(lname); | ||
| QMessageBox::warning( | ||
| this, | ||
| tr("Mumble \u2014 Unregistered user"), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has to be one of the funniest lines of code ever to be submitted to any project ever |
||
| tr("The user \"%1\" is not registered on this server. " | ||
| "ACL entries for unregistered users are discarded by the server and will have no effect. " | ||
| "Please register this user before adding them to an ACL.") | ||
| .arg(name)); | ||
| } | ||
| } | ||
|
|
||
| refillGroupInherit(); | ||
| refillGroupRemove(); | ||
| refillGroupAdd(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ class ACLEditor : public QDialog, public Ui::ACLEditor { | |
| QHash< int, QString > qhNameCache; | ||
| QHash< QString, int > qhIDCache; | ||
| QHash< QString, int > qhNameWait; | ||
| QSet< QString > qsWarnedUnregistered; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use std containers wherever possible in newly introduced code |
||
|
|
||
| int iUnknown; | ||
|
|
||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have either asked a LLM to generate the translation files or squashed the translation commits into your feature commit. Both is unacceptable. |
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.
The premise of this is bad: I don't want to read a message box for all unregistered users. A single user at a time would be ok. Imaging having to click through a dozen message boxes before being able to fix the problem.
Even worse, it looks like the ACLEditor might query users one at the time. Each time we receive an answer all unregistered ones produce message boxes.