3
\$\begingroup\$

Update from Paper label system in Python

I have not updated the security on the app, need to have a proper look into it.

Have decided to use docx module, instead of using print-screen.

I have added few important if statements when handling .docx files in folders depending what the user has clicked etc..

My current issues are:

  • Security of the application
  • Storing username paths inside code isn't a great idea, but it will do for now
  • Will be moving from grid to pack as I don't like the whole feeling of the app itself.
  • Handling the options menu if a user has signed the document, do not show the file that has been signed for that specific user.

I will work on the above, but before I proceed any further. I'd like to hear some feedback on my updated code.

Code below:

import tkinter as tk
from tkinter import messagebox, Frame, Label, Entry, Button, Toplevel, IntVar, Radiobutton, StringVar, OptionMenu
import tkinter.ttk as ttk
import os
import glob
from PIL import Image, ImageTk, ImageGrab
from pathlib import Path
import pyautogui
from time import gmtime, strftime
from docx import Document
import pandas as pd
from collections import Counter


def main():
    root = tk.Tk()

    # Gets the requested values of the height and widht.
    windowWidth = root.winfo_reqwidth()
    windowHeight = root.winfo_reqheight()
    print("Width",windowWidth,"Height",windowHeight)

    # Gets both half the screen width/height and window width/height
    positionRight = int(root.winfo_screenwidth()/2 - windowWidth/2)
    positionDown = int(root.winfo_screenheight()/2 - windowHeight/2)

    # Positions the window in the center of the page.
    root.geometry("+{}+{}".format(positionRight, positionDown))
    app = Window1(root)
    root.mainloop()

#Log in system
class Window1:
    def __init__(self,master):
        self.master = master
        self.master.title("User Log In")
        self.master.geometry('400x150')
        self.frame = Frame(self.master)
        self.frame.pack(fill="both", expand=True)

        #Press enter key to log in
        self.master.bind('<Return>',self.parse)

        #Input for username & password
        self.label_username = Label(self.frame, text="Username: ",font=("bold",16))
        self.entry_username = Entry(self.frame, font = ("bold", 14))
        self.label_password = Label(self.frame, text="Password: ",font=("bold",16))
        self.entry_password = Entry(self.frame, show="*", font = ("bold", 14))
        #layout for inputs
        self.label_username.pack()
        self.entry_username.pack()
        self.label_password.pack()
        self.entry_password.pack()
        #Log in button
        self.logbtn = Button(self.frame, text="Login", font = ("bold", 10), command=self._login_btn_clicked)
        self.logbtn.pack()

    #close and stop tkinter running in backround, also see line #67
    def on_closing(self):
        self.master.destroy()



    def _login_btn_clicked(self):
        # retriving username and password from above entries
        username = self.entry_username.get()
        password = self.entry_password.get()
        found_username = False
        #opening Window2
        def Window2_open():
            self.master.withdraw()
            self.newWindow = Toplevel(self.master)
            self.newWindow.protocol("WM_DELETE_WINDOW", self.on_closing)
            self.app = Window2(self.newWindow, window1 = self)
            #self.newWindow.state('zoomed')
        #password file, will need to improve security
        with open('//SERVER/shared_data/Technical/EdGzi/Sign off/passwords.csv', 'r') as passwords_file:
            for line in passwords_file:
                username_file, password_file = line.strip().split(',')

                if username == username_file:
                    found_username = True
                    if password == password_file:
                        Window2_open() #open main window
                    else:
                        messagebox.showinfo("User message", "Invalid username or password specified please try again")
                    break
        if not found_username:
            messagebox.showinfo("User message", "Invalid username or password specified please try again")


    #function to press enter on keyboard to log in
    def parse(self,event):
        self._login_btn_clicked()

#Main window
class Window2:
    def __init__(self,master, window1):
        #Seperated into tabs
        notebook = ttk.Notebook(master)

        notebook.pack(expand = 1, fill = "both")
        #Frames
        main = ttk.Frame(notebook)
        manual = ttk.Frame(notebook)
        pswd_change = ttk.Frame(notebook)

        notebook.add(main, text='Main-Screen')#Main screen


        notebook.add(manual, text='Manual')#Link to docx files
        notebook.add(pswd_change, text = "Change Password")
        self.window1 = window1 # returning functions from window1


        #validating which user has signed
        username = self.window1.entry_username.get()
        self.User = Label(main, text = 'User: '+ username, font = ('15'))
        self.User.grid(column = 5, row = 0)

        self.info = ["Brand (Logo)", "Product Name:",
                "Product Sub Description: ",
                "Ingredients present in full (any allergens in bold with allergen warning if necessary)",
                "May Contain Statement.",
                "Cocoa Content (%).",
                "Vegetable fat in addition to Cocoa butter",
                "Instructions for Use.",
                "Additional warning statements (pitt/stone, hyperactivity etc)",
                "Nutritional Information Visible",
                "Storage Conditions",
                "Best Before & Batch Information",
                "Net Weight & Correct Font Size.",
                "Barcode - Inner",
                "Address & contact details correct"
                 ]

        self.vars = []
        for idx,i in enumerate(self.info):
            self.var = IntVar(value=0)
            self.vars.append(self.var)
            self.lblOption = Label(main,text=i)
            self.btnYes = Radiobutton(main, text="Yes", variable=self.var, value=2)
            self.btnNo = Radiobutton(main, text="No", variable=self.var, value=1)
            self.btnNa = Radiobutton(main, text="N/A", variable=self.var,value=0)
            self.lblOption.grid(column=4,row=idx, sticky = 'w')
            self.btnYes.grid(column=1,row=idx)
            self.btnNo.grid(column=2,row=idx)
            self.btnNa.grid(column=3,row=idx)

        #paths to sign and signed
        to_sign = '//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/To Sign/'
        signed = '//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Signed/'
        adjust = '//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/Adjust/'

        #Writing to docx file
        def var_states():
            document = Document()
            section = document.sections[0]
            document.add_paragraph(f'Date and Time: {strftime("%d-%m-%Y, %H:%M:%S", gmtime())}')
            document.add_paragraph(f'Sign Off - Name: {username}')
            document.add_picture(f'{self.p}')

            #add table
            table = document.add_table(1, 4)
            #style table
            table.style = 'Table Grid'

            #populate header row
            heading_cells = table.rows[0].cells
            heading_cells[0].text = "Options"
            heading_cells[1].text = self.btnYes.cget("text")
            heading_cells[2].text = self.btnNo.cget("text")
            heading_cells[3].text = self.btnNa.cget("text")

            for idx, item in enumerate(self.vars):
                cells = table.add_row().cells
                cells[0].text = self.info[idx]  # gets the option name
                val = item.get()  #radiobutton value
                if val == 2:  # checks if yes
                    cells[1].text = "*"
                elif val == 1:   # checks if no
                    cells[2].text = "*"
                elif val == 0:   # checks if N/A
                    cells[3].text = "*"

            no_in_result = any([self.var.get() == 1 for self.var in self.vars])
            doc_name = f"{adjust}{os.path.basename(self.p).strip('.jpg') + ' ' + username}.docx" if no_in_result else f"{signed}{os.path.basename(self.p).strip('.jpg') + ' ' + username}.docx"
            fn = document.save(doc_name)
            print(fn)





            #Usernames to verify 2 word documents exist
            ed = (signed+ os.path.basename(self.p).strip('.jpg') +' ed'+ '.docx')
            neal = (signed + os.path.basename(self.p).strip('.jpg') +' neal'+ '.docx')
            jurate = (signed + os.path.basename(self.p).strip('.jpg') +' jurate'+ '.docx')
            karolina = (signed + os.path.basename(self.p).strip('.jpg') +' karolina'+ '.docx')
            rita = (signed + os.path.basename(self.p).strip('.jpg') +' rita'+ '.docx')


            if os.path.exists(neal) and os.path.exists(jurate) == True:
                os.remove(to_sign + os.path.basename(self.p))
                messagebox.showinfo("MSG[T]", "Saved!")
                print("Both users signed the document")
            elif os.path.exists(neal) and os.path.exists(karolina) == True:
                os.remove(to_sign + os.path.basename(self.p))
                messagebox.showinfo("MSG[T]", "Saved!")
                print("Both users signed the document")
            elif os.path.exists(jurate) and os.path.exists(karolina) == True:
                os.remove(to_sign + os.path.basename(self.p))
                messagebox.showinfo("MSG[T]", "Saved!")
                print("Both users signed the document")
            elif os.path.exists(rita) and os.path.exists(karolina) == True:
                os.remove(to_sign + os.path.basename(self.p))
                messagebox.showinfo("MSG[T]", "Saved!")
                print("Both users signed the document")
            elif os.path.exists(rita) and os.path.exists(neal) == True:
                os.remove(to_sign + os.path.basename(self.p))
                messagebox.showinfo("MSG[T]", "Saved!")
                print("Both users signed the document")
            elif os.path.exists(rita) and os.path.exists(jurate) == True:
                os.remove(to_sign + os.path.basename(self.p))
                messagebox.showinfo("MSG[T]", "Saved!")
                print("Both users signed the document")

            else:
                print("One user has signed")
                messagebox.showinfo("MSG[F]", "Document Saved!")

        self.dataSend = Button(main, text = "Send", command = var_states) #send all relevant data from var_states
        self.dataSend.grid(column = 1, row = 16, sticky = 'w')





###################################################################################################################################
                                                        ##Load Image##
###################################################################################################################################
        try:
            # Create a Tkinter variable
            self.tkvar = StringVar()

            # Directory
            self.directory = '//SERVER/shared_data/Technical/Label Sign Off Sheets/sign off project/To Sign/'
            self.choices = os.listdir(self.directory)
            #self.choices = glob.glob(os.path.join(self.directory, "*")) #all choices
            self.tkvar.set('...To Sign Off...') # set the default option

###for later###             ###for later###
            # for subdir, dirs, files in os.walk(signed):
            #     for file in files:
            #         x = os.path.join(file).split()[-3:-1]
            #         y = ' '.join(x)
            #
            #
            # if os.path.join(subdir, file).endswith(f'{username}.docx') is True: #if this file exists, do not show 'to-sign' file for current user signed in
            #     self.choices = glob.glob(os.path.join(self.directory, '{y}*.jpg'.format(prefix=y)))
            #     print("true")
###for later###             ###for later###





            # Images, placing the image onto canvas
            def change_dropdown():
                imgpath = self.tkvar.get()
                img = Image.open(self.directory + imgpath)
                photo = ImageTk.PhotoImage(img)
                label2.image = photo
                label2.configure(image=photo)

            #return path value of selected directory
            self.p = None
            def func(value):
                global p
                self.p = Path(self.directory + value)
                print(self.p)

            #reset function to continue signing other labels
            def reset():
                for intvar in self.vars:
                    intvar.set(0)

                label2.configure(image='')
                self.tkvar.set('...To Sign Off...') # set the default option


            #widgets
            self.msg1 = Label(main, text = "Choose here")
            self.msg1.grid(column = 0, row = 0)
            self.popupMenu = OptionMenu(main, self.tkvar, *self.choices, command = func) #Dropdown menu of all sign off Sheets that need signing
            self.popupMenu.grid(row=1, column=0)
            self.display_label = label2 = Label(main, image=None)
            self.display_label.grid(row=2, column=0, rowspan = 500)
            self.open_button = Button(main, text="Open", command=change_dropdown) # opens the directory and opens selected image
            self.open_button.grid(row=502, column=0)
            self.resetBtn = Button(main, text = "reset", command = reset)
            self.resetBtn.grid(column = 1, row = 17, sticky = 'w')

        except TypeError: #if no images found in folder, then show messagebox & disable send btn
            self.dataSend['state'] = 'disabled'
            messagebox.showinfo("Label Error", "No labels to sign, please quit the program!")


###################################################################################################################################
                                                ##Click and drag for measuring purposes##
###################################################################################################################################
        # def drag(event):
        #     event.widget.place(x=event.x_root, y=event.y_root,anchor=CENTER)
        #
        #
        #
        # self.card = Canvas(main, width=50, height=50, bg='blue')
        # self.card.place(x=300, y=600,anchor=CENTER)
        #

        # self.card.bind("<B1-Motion>", drag)





###################################################################################################################################
                                                        ##TAB 2 - MANUAL##
###################################################################################################################################

        #opening a docx file for manual guide on how to print labels.
        def manualopen():
            # file_to_open = str(Path("//SERVER/shared_data/Technical/Food Safety & Quality Manual/21.LABL.02 - Labelling notes.docx"))
            # subprocess.check_call(['open', file_to_open])
            file_to_open = r"\\SERVER\shared_data\Technical\Food Safety & Quality Manual\Section 21 - Process Control\21.LABL.02 - Labelling notes.docx"
            os.startfile(file_to_open)



        self.manualBtn = Button(manual, text= "open doc", command = manualopen)
        self.manualBtn.pack()




###################################################################################################################################
                                                        ##TAB 3 - Password Change##
###################################################################################################################################

        self.usernameReturn = Label(pswd_change, text = "Username: "+username).pack()
        self.msgPass = Label(pswd_change, text = "New Password: ").pack()
        self.password_change = Entry(pswd_change, show = "*")
        self.password_change.pack()

        def changepass():
            password_replace = pd.read_csv("//SERVER/shared_data/Technical/EdGzi/Sign off/passwords.csv")
            password_replace.loc[password_replace["username"]==username, "pa55w07541146ffdf4s65"] = self.password_change.get()

            password_replace.to_csv("//SERVER/shared_data/Technical/EdGzi/Sign off/passwords.csv", index=False)
            messagebox.showinfo("Password MSG", "Password Changed!")

        self.btnChangePass = Button(pswd_change, text = "Change Password", command = changepass)
        self.btnChangePass.pack()




if __name__ == '__main__':
    main()
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

A few other things that weren't mentioned so far:

In a few places, you do something like this:

f"{adjust}{os.path.basename(self.p).strip('.jpg') + ' ' + username}"

Why not simply:

f"{adjust}{os.path.basename(self.p).strip('.jpg')} {username}"

Speaking of which, your use of str.strip doesn't behave the way you think it does. Take a look:

>>> help(str.strip)
Help on method_descriptor:

strip(self, chars=None, /)
    Return a copy of the string with leading and trailing whitespace remove.

    If chars is given and not None, remove characters in chars instead.

>>> 

In your case, the optional chars parameter is not None, it's ".jpg". That means it will remove any of the characters specified in chars. Not only that, but since you're using str.strip instead of str.rstrip, you're potentially removing those characters not just from the back, but from the front as well.

For example:

>>> "john_help.jpg".strip(".jpg")
'ohn_hel'
>>>

Definitely not the desired output in your case. Using str.rstrip would strip only from the back of the string, but it still wouldn't give you the desired result:

>>> "john_help.jpg".rstrip(".jpg")
'john_hel'
>>>

If you're using Python 3.4+, you're better of using pathlib anyway instead of all that os.path stuff:

>>> from pathlib import Path
>>> Path("root/dir/sub/file.jpg").stem
'file'
>>> 

I would also suggest using an enum.Enum to represent the possible values / options / states of your tk.Radiobuttons. It prevents you from "stringify-ing" the options or doing something like this:

val = item.get()  #radiobutton value
if val == 2:  # checks if yes
    cells[1].text = "*"
elif val == 1:   # checks if no
    cells[2].text = "*"
elif val == 0:   # checks if N/A
    cells[3].text = "*"

Here's how I might set up a tk.Radiobutton using an enum:

import tkinter as tk


class Application(tk.Tk):

    from enum import Enum


    class RadioOption(Enum):
        Red = 0
        Green = 1
        Blue = 2

    def __init__(self, *args, **kwargs):

        tk.Tk.__init__(self, *args, **kwargs)
        self.title("Title")
        self.geometry("100x100")
        self.resizable(width=False, height=False)

        self.radio_variable = tk.Variable(None, Application.RadioOption.Red)

        def on_radio_variable_change(*args):
            print(self.radio_variable.get())
        self.radio_variable.trace(mode="w", callback=on_radio_variable_change)

        self.radio_button_red = tk.Radiobutton(
            self,
            text="Red",
            variable=self.radio_variable,
            value=Application.RadioOption.Red,
        )

        self.radio_button_green = tk.Radiobutton(
            self,
            text="Green",
            variable=self.radio_variable,
            value=Application.RadioOption.Green,
        )

        self.radio_button_blue = tk.Radiobutton(
            self,
            text="Blue",
            variable=self.radio_variable,
            value=Application.RadioOption.Blue,
        )

        self.radio_button_red.pack(anchor=tk.W)
        self.radio_button_green.pack(anchor=tk.W)
        self.radio_button_blue.pack(anchor=tk.W)

def main():

    application = Application()
    application.mainloop()

    return 0


if __name__ == "__main__":
    import sys
    sys.exit(main())

And one more tiny nit-pick, in a few different places you do something like this:

self.open_button.grid(row=502, column=0)
self.resetBtn = Button(main, text = "reset", command = reset)

See how some of the keyword-arguments have additional whitespace, and others do not? Pick one style, but don't do both - personally I would remove the whitespace since that's PEP8 compliant.

\$\endgroup\$
2
\$\begingroup\$

Dimension output

This:

# Gets the requested values of the height and widht.
windowWidth = root.winfo_reqwidth()
windowHeight = root.winfo_reqheight()
print("Width",windowWidth,"Height",windowHeight)

firstly has a spelling mistake - widht = width. That aside, you can probably just simplify it to

window_width = root.winfo_reqwidth()
window_height = root.winfo_reqheight()
print(f'Dimensions: {window_width}x{window_height}')

Also note the use of lower_camel_case.

Similarly,

root.geometry(f"+{position_right}+{position_down}")

Class names

Pick better names than Window1, Window2. Perhaps MainWindow, LoginWindow.

Password storage

Other than this being insecure - which you've already identified - there are other issues:

    with open('//SERVER/shared_data/Technical/EdGzi/Sign off/passwords.csv', 'r') as passwords_file:
        for line in passwords_file:
            username_file, password_file = line.strip().split(',')

            if username == username_file:
                found_username = True
                if password == password_file:
                    Window2_open() #open main window
                else:
                    messagebox.showinfo("User message", "Invalid username or password specified please try again")
                break

Don't hard-code that path. At the least, use ~ and resolve your home directory, and/or accept a string parameter in the constructor. Since you're manipulating multiple files, establish a supported directory structure, and parametrize the base path - in your case, //SERVER/shared_data/Technical.

You set found_username to True even if the password is incorrect. That doesn't seem right. Beyond that: you shouldn't need to use a found flag at all - just break if you find the thing, and write a for/else to detect if you didn't break.

In general, you should try harder to separate your presentation (i.e. messagebox) from your logic (i.e. password storage, retrieval and comparison). Entirely different classes, maybe different modules.

Single-parameter formatting

Don't do this:

        document.add_picture(f'{self.p}')

instead,

        document.add_picture(str(self.p))
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.