chore: Code improvements and fixes by 0nly-Fir3 · Pull Request #302 · DouglasHalse/snackAttackTrack

0nly-Fir3 · 2025-12-12T10:26:03Z

Summary

Various code improvements and fixes across the codebase.

Changes

Updated test configurations
Profile screen improvements
Create user screen updates
Custom screen manager enhancements
Main user screen improvements
Settings manager updates
Splash screen refinements
Top up payment screen updates
Navigation header improvements
README updates
Workflow and gitignore updates

Includes various code improvements: - Updated test configurations - Profile screen improvements - Create user screen updates - Custom screen manager enhancements - Main user screen improvements - Settings manager updates - Splash screen refinements - Top up payment screen updates - Navigation header improvements - README updates - Workflow and gitignore updates

Copilot

Pull request overview

This pull request introduces PIN authentication and guest mode functionality to the Snack Attack Track subscription management software, along with various UI improvements and code quality enhancements.

Key Changes:

Added 4-digit PIN authentication for user accounts with validation logic
Implemented guest mode allowing users to browse without an account
Enhanced UI/UX with improved top-up payment screen design and loading states
Updated test configurations and GitHub Actions workflows

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
README.md	Added comprehensive documentation for PIN authentication, admin panel access, and Raspberry Pi deployment instructions
CLEANUP.md	New file documenting code cleanup activities and project structure
GuiApp/widgets/uiElements/navigationHeader.py	Integrated admin PIN popup for settings access and added guest mode display indicators
GuiApp/widgets/topUpPaymentScreen.py	Added loading state UI and amount display for payment processing
GuiApp/widgets/splashScreen.py	Added clarifying comment about card login flow
GuiApp/widgets/settingsManager.py	Added LOW_INVENTORY_THRESHOLD setting enum
GuiApp/widgets/mainUserScreen.py	Implemented guest mode restrictions for buy, top-up, profile, and gamble features
GuiApp/widgets/customScreenManager.py	Added guest mode login functionality and helper methods with timer cancellation fix
GuiApp/widgets/createUserScreen.py	Implemented PIN validation (empty check, length, digits, matching) and registration flow updates
GuiApp/widgets/ProfileScreen.py	Added change PIN option with corresponding button handler
GuiApp/tests/test_create_user.py	Updated all user creation tests to include PIN inputs
GuiApp/tests/conftest.py	Added sys.path manipulation for imports and updated fixtures with PIN parameter
GuiApp/kv/uiElements/textInputs.kv	Added theme color configuration for virtual keyboard
GuiApp/kv/topUpPaymentScreen.kv	Complete UI redesign with improved layout, warning messages, and step-by-step instructions
GuiApp/kv/main.kv	Added includes for new popup KV files (PIN entry, admin PIN, change PIN, low inventory alert)
GuiApp/kv/createUserScreen.kv	Added PIN and confirm PIN input fields with password masking
GuiApp/kv/ProfileScreen.kv	Added ChangePinOption button and updated grid layout to 3 columns
.gitignore	Expanded patterns to exclude test outputs and PNG files (with negation for Images folder)
.github/workflows/VisualDiff.yml	Updated Python version to 3.13 and replaced deprecated set-output syntax

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-12T10:38:22Z

        page_name: "Profile"
    GridLayout:
        id: optionsGrid
+        cols: 3


The grid layout was changed from implicit column sizing to explicitly setting cols: 3 to accommodate the new ChangePinOption. However, this hardcoded value reduces flexibility. If more options are added or removed in the future, the column count will need manual adjustment. Consider using dynamic sizing or document this constraint in a comment.

Suggested change

cols: 3

# The number of columns will automatically match the number of option widgets.

Copilot · 2025-12-12T10:38:22Z

            timeToAutoLogout = self.settingsManager.get_setting_value(
                settingName=SettingName.AUTO_LOGOUT_ON_IDLE_TIME
            )
+            if self.log_out_timer:


The timer cancellation at line 57 could potentially cause issues if self.log_out_timer is None. While the code sets it later, if login() is called before any timer is created, this could raise an AttributeError. Consider adding a null check before cancelling the timer.

Suggested change

if self.log_out_timer:

if self.log_out_timer and hasattr(self.log_out_timer, "cancel"):

Copilot · 2025-12-12T10:38:23Z

+            if (
+                hasattr(app.screenManager, "is_guest_mode")
+                and app.screenManager.is_guest_mode()
+            ):
+                self.ids.welcomeTextLabel.text = f"Welcome, {full_name} (Guest Mode)"
+            else:
+                self.ids.welcomeTextLabel.text = f"Welcome, {full_name}"
+        else:
+            self.ids.welcomeTextLabel.text = "Welcome, Guest"
        if self.page_name:
            self.ids.welcomeTextLabel.text += f" - {self.page_name}"

    def set_current_credits(self, current_credits):
-        self.ids.patronCreditsLabel.text = f"Your credits: {current_credits:.2f}"
+        app = App.get_running_app()
+        if (
+            hasattr(app.screenManager, "is_guest_mode")
+            and app.screenManager.is_guest_mode()
+        ):


The guest mode check is repeated in lines 81-84 and lines 95-98. Consider extracting this into a helper method to reduce duplication and improve maintainability. For example, a method like _is_guest_mode() could centralize this logic.

Copilot · 2025-12-12T10:38:23Z

+        if pin == "":
+            ErrorMessagePopup(errorMessage="PIN is required for security").open()
+            return
+
+        if len(pin) != 4:
+            ErrorMessagePopup(errorMessage="PIN must be exactly 4 digits").open()
+            return
+
+        if not pin.isdigit():
+            ErrorMessagePopup(errorMessage="PIN must contain only numbers").open()
+            return
+
+        if pin != confirmPin:
+            ErrorMessagePopup(errorMessage="PINs do not match").open()
+            return


The PIN validation only checks length and digit constraints but doesn't prevent weak PINs like "1234", "0000", or repeating digits. For a 4-digit PIN, these are extremely common and easy to guess. Consider adding validation to reject common patterns like sequential digits (1234, 4321), repeating digits (1111, 0000), or maintaining a blacklist of commonly used PINs.

Copilot · 2025-12-12T10:38:23Z

+                        radius: [10,]
+                Label:
+                    bold: True
+                    text: "✅ Step 3:"


The warning message uses emoji characters (⚠️, 🔍, 💸, ✅) which may not render correctly on all systems, particularly on Raspberry Pi with limited font support or when using the OS Lite configuration mentioned in the README. Consider using text-based alternatives or ensuring the target system has appropriate emoji font support.

Suggested change

text: "✅ Step 3:"

text: "Step 3: Confirm"

Copilot · 2025-12-12T10:38:24Z

    def onBuyButtonPressed(self, _):
+        if self.manager.is_guest_mode():
+            ErrorMessagePopup(
+                errorMessage="Please create an account to purchase items"
+            ).open()
+            return
        self.manager.transitionToScreen("buyScreen")

    def onTopUpButtonPressed(self, _):
+        if self.manager.is_guest_mode():
+            ErrorMessagePopup(
+                errorMessage="Please create an account to top up credits"
+            ).open()
+            return
        self.manager.transitionToScreen("topUpAmountScreen")

    def onProfileButtonPressed(self, _):
+        if self.manager.is_guest_mode():
+            ErrorMessagePopup(
+                errorMessage="Please create an account to view profile"
+            ).open()
+            return
        self.manager.transitionToScreen("profileScreen")

    def onGambleButtonPressed(self, _):
+        if self.manager.is_guest_mode():
+            ErrorMessagePopup(
+                errorMessage="Please create an account to use the wheel of snacks"
+            ).open()


The guest mode check logic is duplicated across four different button press handlers (lines 33-36, 41-45, 49-53, 57-61). This violates the DRY principle. Consider creating a decorator or helper method that checks guest mode before executing button handlers, which would reduce code duplication and improve maintainability.

Suggested change

def onBuyButtonPressed(self, _):

if self.manager.is_guest_mode():

ErrorMessagePopup(

errorMessage="Please create an account to purchase items"

).open()

return

self.manager.transitionToScreen("buyScreen")

def onTopUpButtonPressed(self, _):

if self.manager.is_guest_mode():

ErrorMessagePopup(

errorMessage="Please create an account to top up credits"

).open()

return

self.manager.transitionToScreen("topUpAmountScreen")

def onProfileButtonPressed(self, _):

if self.manager.is_guest_mode():

ErrorMessagePopup(

errorMessage="Please create an account to view profile"

).open()

return

self.manager.transitionToScreen("profileScreen")

def onGambleButtonPressed(self, _):

if self.manager.is_guest_mode():

ErrorMessagePopup(

errorMessage="Please create an account to use the wheel of snacks"

).open()

def _check_guest_mode_and_show_error(self, error_message):

if self.manager.is_guest_mode():

ErrorMessagePopup(errorMessage=error_message).open()

return True

return False

def onBuyButtonPressed(self, _):

if self._check_guest_mode_and_show_error("Please create an account to purchase items"):

return

self.manager.transitionToScreen("buyScreen")

def onTopUpButtonPressed(self, _):

if self._check_guest_mode_and_show_error("Please create an account to top up credits"):

return

self.manager.transitionToScreen("topUpAmountScreen")

def onProfileButtonPressed(self, _):

if self._check_guest_mode_and_show_error("Please create an account to view profile"):

return

self.manager.transitionToScreen("profileScreen")

def onGambleButtonPressed(self, _):

if self._check_guest_mode_and_show_error("Please create an account to use the wheel of snacks"):

Copilot · 2025-12-12T10:38:25Z

+            patronId=-1,
+            firstName="Guest",
+            lastName="User",
+            totalCredits=0.0,
+            employeeID=0,
+            pin=None,
+        )
+        self.logged_in_user = self._currentPatron
+        # No auto-logout timer for guest mode
+
+    def is_guest_mode(self):
+        """Check if current user is in guest mode"""
+        return self._currentPatron is not None and self._currentPatron.patronId == -1


Using patronId=-1 as a magic number to identify guest mode is a potential maintainability issue. Consider using a constant like GUEST_USER_ID = -1 defined at the module level, or better yet, add an explicit is_guest boolean field to the UserData object to make the intent clearer.

Copilot · 2025-12-12T10:38:25Z

    def onConfirm(self, *largs):
+        # Show loading state
+        self.ids["loadingLabel"].text = "Processing payment..."
+        self.ids["confirmButton"].disabled = True
+
        self.manager.database.addTopUpTransaction(
            patronID=self.userData.patronId,
            amountBeforeTransaction=self.userData.totalCredits,


Missing input validation: The payment can be confirmed without verifying that the payment was actually received. The code adds a loading state but doesn't appear to include any validation to check if the Swish payment was successful before crediting the user's account. This could allow users to add credits without actually paying, which is a critical security vulnerability.

Copilot · 2025-12-12T10:38:25Z

+### Admin Panel Access
+
+The admin panel is protected by a separate admin PIN for enhanced security:
+- **Admin PIN**: `4444` (default)


The default admin PIN is hardcoded in the README and documented as "4444". This is a security risk as it exposes the default admin credentials publicly in the repository. Consider:

Using a randomly generated PIN on first setup

Forcing admin PIN change on initial setup

At minimum, removing the specific PIN value from the README and referring users to the installation documentation for obtaining the initial PIN

Suggested change

- **Admin PIN**: `4444` (default)

- **Admin PIN**: See installation documentation for initial PIN setup

Copilot · 2025-12-12T10:38:25Z

+            lastName="User",
+            totalCredits=0.0,
+            employeeID=0,
+            pin=None,


Keyword argument 'pin' is not a supported parameter name of UserData.init.

Suggested change

pin=None,

Copilot AI review requested due to automatic review settings December 12, 2025 10:26

Copilot started reviewing on behalf of 0nly-Fir3 December 12, 2025 10:26 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

Navigation Menu

chore: Code improvements and fixes - #302

chore: Code improvements and fixes#302
0nly-Fir3 wants to merge 1 commit into
DouglasHalse:mainfrom
0nly-Fir3:chore/code-improvements

0nly-Fir3 commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Copilot AI Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	cols: 3
	# The number of columns will automatically match the number of option widgets.

	if self.log_out_timer:
	if self.log_out_timer and hasattr(self.log_out_timer, "cancel"):

	- Admin PIN: `4444` (default)
	- Admin PIN: See installation documentation for initial PIN setup

Conversation

0nly-Fir3 commented Dec 12, 2025

Summary

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants