Clean up code #8
Labels
No Label
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Unraid
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: FrederikBaerentsen/BrickTracker#8
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Especially app.py
Hello,
thank you for making this app. I wanted to add some features that would be useful to me (like who owns the set, where it is stored) but the massive
app.py
scared me.I started rewriting some parts into objects, and made a bit more modular.
I also started rewriting some parts of the UI to my taste (which is my very own 😅, and is very much the default Bootstrap 5 boilerplate).
It looks like this so far on my side:
I'll share it in a branch if you are interested, once I'm on par with your version in terms of features.
Happy new year!
I’m very open for new features! And who own a set (eg you or a child) would be nice. The same with the location it is stored.
The UI changes are great! Please open a PR as I love the changes you have made.
Hello, just to keep you informed, I'm still processing all your code 🙃.
I've re-implemented the sorting/filtering/searching through the sets.
It looks like this so far:
That looks great. Unfortunately i haven't had time to work on this over the holidays. I hope to get back to it soon though. Do you want me to add you as collaborator and make a branch for your design?
Don't worry too much about it, I'll make my own fork whenever and do the proper merge request process.
It's going to take a while longer because I'm rewriting everything I see (whenever I can make sense of it).
For instance so far the
app.py
file looks like this now:That's all, and it's not going to grow much besides a couple routes 😁.
Got the set parts listing redone, with saving when exiting the input field rather than clicking the save button.
This should cover #29.
Also even if it is in an
input
field, sorting by number of missing pieces is valid for parts now.Looks great. Looking forward to seeing the code as I'm sure i have lots to learn. Showed your design to my wife and she said it was so much better than mine.
Minifigures are in. I can now get to what I was dreading: your create page 😂.
Basically, most of the create page is just socket.io to send a hacked together status bar back and forth. Honestly it could be skipped.
My rough workflow is like:
Check if the input number contains
-
. If it does, assume the full set number is what’s in the input field (eg 79000-1 or 71039-7).If it doesn’t contain
-
, append-1
to the number.Check the input number against the downloaded sets.csv from rebrickable. If it isn’t in that file, reject it and say it doesn’t exist.
Use the rebrick library to get set info, then inventory, then minifig info. I think the rebrickable api can return multiple things in one query but I didn’t look into that.
Save to sql and download all images that doesn’t exists already.
Return to start page.
The progress bar checks how many images needs downloaded and makes an estimate of the progress. But it’s very bad and I wish I hadn’t spend so much time on it 😂
After typing my whole spiel out I could have just started here.
Watching closely, but from my end the more modular (at an html/class level) the better for any CSS tuning. My budibase 'app' looks like this and I'll be trying to emulate it here eventually as I just can't build the features I want
Reworked adding a set 🤯:
sets.csv
to check if a set exists-2
sets)-1
if no version has been suppliedAdded visual feedback when using the set checkboxes. This is mentioned in #35.
Reworked the deletion workflow, with a confirmation screen.
Added a visual indicator for the socket. This could help with situations like #33.
Great job so far. I've not started my code cleanup and issue fixing yet, as it would just be double the work. Looking forward to testing your version!
Reworked gracefully handling
app.db
not initialized.Added a lightweight bulk add. It does nothing fancy, and does not resume on error.
It takes in entry a list of sets. On error it put backs into the input whatever was not processed (including the errored set).
This should help with part of #25 🙂.
Added a very basic (and optional) read-only/authenticated mode (with a unique password) to be able to share the page read-only.
It blocks:
Added part detail (a bit janky because I had a hard time understanding what makes a part unique, I'm no LEGO expert 😅)
It displays which sets are using this part. It's displayed from clicking on a part in the listing
Thinking it's a bit risky, would rather do it using reverse proxy. Implementing proper authentication is hard :)
I think i did a mix of element ID and color.
Really loving the design and how you highlight various things!
Just to reassure you, I didn't produce the authentication code myself. I'm using the existing
flask-login
package.I called it basic only because the user database is only one user, and its password is set through an env variable.
But it does use proper sessions, isolation between clients, cryptographically signed sessions and cookies etc... 🙂
That's great to hear. Thought you had implemented it yourself 👍
Added minifigures list and details
Added "normal" logs when not in
DEBUG=true
Including database actions:
Displaying all info about the configuration variables in the admin panel, including their values (except for secrets), if they are changed from their defaults, and which environment variable controls it
Reworked how instructions are presented (without the dropdown). Made the name matching more lenient and also added some admin view.
Added a global list of files in
instructions
Added list of last or random (if using the
RANDOM
env variable) sets and minifigures on the front page.Reworked instructions upload, finished lightweight management of instructions.
Added gracefully handling missing
themes.csv
and light management of the fileis there an idea of when a version of this planned to be available for bug testing/functionality tests etc?
I'm working on this on my free time, but the only task left I have are:
Hopefully it is accepted because my version is a massive departure from @FrederikBaerentsen original code, even if it should be compatible with existing data, I haven't changed any of the database or filesystem structure. You could say it is a completely different app even if it has the same behavior 🙂
I don't have an ETA but it should be soon™
Reworked the wishlist and retired sets date
Sounds great! I am really excited. I am thinking I will retire my code on a separate branch and make your new code the "main code". I'll setup a docker build system, so we don't have to manually build and push this too.
I don't think you need a new branch to keep your old code. You have the git history doing that job 🙂.
My merge request will clean up the old files since I've removed them one by one after re-implementing the code 👍.
So I currently have your version running with a few sets.
I've build a new image with my code.
Then adjusted my compose for testing
And it seems to work pretty much as a drop-in replacement 🙂
The
nil
andnil_mf
files have moved so there a simple fix to apply to put them where they should beI also tortured it to test the Bulk add function
The list
Midway through
Finished
I still have a few quirks and wrong sums to fix and I'll be finished with this first version
I detected a bug in your version which inserts
'None'
instead ofNULL
in the missing table for items without anelement_id
. This leads to "phantom" missing parts. I baked in a web based fix so that users don't have to fiddle with the CLI.Also added the default image refresh while I was at it so that no fix is required through the CLI 😂
FYI I have submitted merge request #40.
If anyone is eager to test you'll have to clone my fork and build it yourself (if you are brave enough).
Tested both local install and using prebuilt image. Made release notes here: https://gitea.baerentsen.space/FrederikBaerentsen/BrickTracker/releases/tag/v1.0.0
All looks good on my test installs and migration is easy. I'll go through the issues this week and close the ones that has been fixed. Great work!
@gregoo Is there a reason why you prefer volumes over local mapping? I always felt like local folders was easier for beginners to use and also to backup.
My data is stored on a NAS, that's why I'm using volumes + NFS.
Local volumes is simply your data stored in
/var/lib/docker
, there is nothing fancy about it.If you have a backup solution, you can simply spawn a container with all the volumes you want to backup mounted, plus the destination where you need to save the data. For the application running inside the container, everything will look local.
On a side note, there is a discrepancy between the tag you applied to your image (
v1.0.0
) and the one in mycompose.yaml
file (1.0.0
). You should probably drop thev
at the beginning if you want to adhere to something like SemVer for the version numbering (https://semver.org/). Otherwise you need to fix the version in the compose file.Alright, I was mainly wondering what would be easiest to have in the sample files. I am thinking of making a Quickstart section of the setup docs with simple copy paste instructions.
Ohh i see. I'll upload a new version without the
v
asap.I think we can close this monster thread unless you see something in it that remains to do 🙂
@gregoo agreed. I was just going through and checking.