-
Notifications
You must be signed in to change notification settings - Fork 51
Refactor (foobar) razzia #495
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
Conversation
Foobar functionality is now implemented. The new razzia-system is hidden at The next step is to implement some user-friendly way to change the razzia settings, even though it actually is possible to do so using the new 'Razzia'-setting in the admin panel TODO:
|
I did a basic review of this and everything seemed to work, but I don't exactly know how the whole razzia thing works, so I am afraid to formally review it :) |
Perfect timing. The basic purpose of the razzia system, is to validate that a member is actually a member. It also has the option for a timer between visits, based on how the Razzia object is defined |
I can approve it formally if you want me to, as long as you are aware that I am maybe not the best person to review it :) I understand the purpose and use of the system, but I am unsure about how it works in the backend, and how it ties into the old razzia system. My lack of knowledge in that area means I probably missed some edge case tests and can't really give any thoughts on whether you designed the code correctly. |
🙏 Normally I wouldn't want people to review if they aren't sure, but I think your experience is already the same as most other reviewers. Few FIT members actually know how the tools are used Should be ready now |
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.
Works on my machine™
The previous razzia code was very difficult to maintain, the goal is to combine Bread, Foobar and Fnugfald razzia into a single generic razzia. Eventually, the razzia wizard functionality can be implemented as well (but that's not a goal for this PR)
This builds on the experience from #464, but replaces it as well.