add password protected tournaments#2544
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds support for password-protected tournaments, allowing users to join private tournaments by entering a password/entry code. The implementation includes comprehensive test coverage for the password entry flow, both for successful joins and incorrect password scenarios.
Changes:
- Added
privatefield to the Tournament data model to identify password-protected tournaments - Updated tournament join API to accept an optional password parameter
- Added password dialog UI that prompts users when joining private tournaments
- Included comprehensive test cases covering password dialog display, successful joins, and error handling
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/src/model/tournament/tournament.dart | Added private boolean field to Tournament model and parsing logic to identify password-protected tournaments |
| lib/src/model/tournament/tournament_repository.dart | Updated join method to accept optional password parameter and include it in the API request body |
| lib/src/model/tournament/tournament_controller.dart | Updated joinOrPause method signature to accept optional password and properly await repository calls |
| lib/src/view/tournament/tournament_screen.dart | Added password dialog and join logic for private tournaments with proper error handling for incorrect passwords |
| test/view/tournament/tournament_screen_test.dart | Added comprehensive test coverage including password dialog display, successful join with correct password, and error handling for incorrect password |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Future<String?> _showPasswordDialog(BuildContext context) { | ||
| final TextEditingController controller = TextEditingController(); | ||
|
|
||
| return showDialog<String>( | ||
| context: context, | ||
| builder: (context) { | ||
| return AlertDialog.adaptive( | ||
| title: Text(context.l10n.tournamentEntryCode), | ||
| content: TextField( | ||
| controller: controller, | ||
| decoration: InputDecoration(hintText: context.l10n.password), | ||
| ), | ||
| actions: [ | ||
| TextButton( | ||
| onPressed: () => Navigator.of(context).pop(), | ||
| child: Text(context.l10n.cancel), | ||
| ), | ||
| TextButton( | ||
| onPressed: () => Navigator.of(context).pop(controller.text), | ||
| child: Text(context.l10n.join), | ||
| ), | ||
| ], | ||
| ); | ||
| }, |
There was a problem hiding this comment.
The TextEditingController is created but never explicitly disposed. While this is acceptable for short-lived dialogs and the controller will be garbage collected, consider wrapping the dialog content in a StatefulWidget to properly dispose of the controller following Flutter best practices, similar to how it's done in other parts of the codebase (e.g., chat_screen.dart).
| Future<String?> _showPasswordDialog(BuildContext context) { | |
| final TextEditingController controller = TextEditingController(); | |
| return showDialog<String>( | |
| context: context, | |
| builder: (context) { | |
| return AlertDialog.adaptive( | |
| title: Text(context.l10n.tournamentEntryCode), | |
| content: TextField( | |
| controller: controller, | |
| decoration: InputDecoration(hintText: context.l10n.password), | |
| ), | |
| actions: [ | |
| TextButton( | |
| onPressed: () => Navigator.of(context).pop(), | |
| child: Text(context.l10n.cancel), | |
| ), | |
| TextButton( | |
| onPressed: () => Navigator.of(context).pop(controller.text), | |
| child: Text(context.l10n.join), | |
| ), | |
| ], | |
| ); | |
| }, | |
| class _PasswordDialog extends StatefulWidget { | |
| const _PasswordDialog(); | |
| @override | |
| State<_PasswordDialog> createState() => _PasswordDialogState(); | |
| } | |
| class _PasswordDialogState extends State<_PasswordDialog> { | |
| late final TextEditingController _controller; | |
| @override | |
| void initState() { | |
| super.initState(); | |
| _controller = TextEditingController(); | |
| } | |
| @override | |
| void dispose() { | |
| _controller.dispose(); | |
| super.dispose(); | |
| } | |
| @override | |
| Widget build(BuildContext context) { | |
| return AlertDialog.adaptive( | |
| title: Text(context.l10n.tournamentEntryCode), | |
| content: TextField( | |
| controller: _controller, | |
| decoration: InputDecoration(hintText: context.l10n.password), | |
| ), | |
| actions: [ | |
| TextButton( | |
| onPressed: () => Navigator.of(context).pop(), | |
| child: Text(context.l10n.cancel), | |
| ), | |
| TextButton( | |
| onPressed: () => Navigator.of(context).pop(_controller.text), | |
| child: Text(context.l10n.join), | |
| ), | |
| ], | |
| ); | |
| } | |
| } | |
| Future<String?> _showPasswordDialog(BuildContext context) { | |
| return showDialog<String>( | |
| context: context, | |
| builder: (context) => const _PasswordDialog(), |
|
I think both the code and the UI should consistently call it an "entry code" or similar and avoid naming it password. Since password comes with expectations around encryption etc. Which is not what the code is intended for, it's for giving to others so they can join. It's why the website renamed it back in 2021 lichess-org/lila#9467 |
Good point, makes sense! |
|
Thanks for the PR. Will review it when I have dealt with older ones. |
| .read(tournamentControllerProvider(widget.state.id).notifier) | ||
| .joinOrPause(entryCode: entryCode); | ||
| } catch (e) { | ||
| setState(() { |
There was a problem hiding this comment.
Better put this in a finally clause.
There was a problem hiding this comment.
If the join request is successful line 1089 will update the joinOrLeaveProgress (which is better than updating it immediately so that it keeps spinning until the green bar appears) so we only need it for the unsuccessful case.
3 participants
closes #1935
video:
tournament_password.mp4
includes testcases