You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As Snipe-IT is a... let's say more mature project, it has obviously gathered some technical debt over the past decade and a half. New features got released in Laravel. Some good, some not so good, some worked well for us, and others we discovered weren't a great fit because of our own unique user base and feature set, and forward-looking goals.
Please note that this is not a place for feature requests - this is 100% code strategy. We already know what the feature requests are. As of this writing, we have over 950 open issues, so I promise that we know what folks are asking for.
Goals
Faster development cycles which enable us to ship features with less risk and at higher velocity
More confidence in new features and changes to existing features
Better testability
Better discoverability - standardizing the way we handle FCOs, logging, validation, etc. Right now we have several different ways of solving the same(ish) problems across models
More reliable results across different methods - the same things should happen (logging, notifications, etc) regardless of whether you take an action via the GUI, the API, CLI, bulk actions, the importer (and whatever comes next)
Clearer paths for outside devs
Lexicon
These are terms you'll see used here since they are used within the organization as a shorthand. We won't be over-explaining Laravel built-ins, as they have their own documentation on that. 😀
"FCO" -> First Class Object - Assets, Users, Locations, etc.
"FMCS" -> Full Multiple Company Support
This project is a lot more complicated than it looks on the face of things, largely due to admin-specific settings, things like FMCS, and the complex permission structure we provide.
Os Elefantes 🐘 🐘 🐘
There are a few elephants in the room that I want to get out of the way quickly.
We need to upgrade the AdminLTE theme (or pick another one) so that we're using a more current version of Bootstrap (or whatever other CSS framework we want to use. No, Tailwind is not currently being seriously considered at this time.)
A big part of our priorities here have always been (and must always be) to keep this project as simple to install as possible. Knowledge and access to a boring LAMP stack MUST be enough to get Snipe-IT running without subscribing to third party services, having special knowledge about queuing or caching systems, etc.
This does occasionally mean we can't add some bells and whistles, or if we do, we have to add them in such a way that they have sane fallbacks that still work without extra services.
We have traits in what feels like a million places. (It's not, it's like 4, but 4 feels like too many for what they do.) We need to have a word with ourselves and decide whether or how we can consolidate some of them so they're easier to find. We've already started some work on this, but there's more to be done.
Available Tooling
As I've been working on this, I realized a better approach is to go through all of our directories and give a quick overview of what were using, what we're not, and what functionality might be duplicated without us realizing it.
Eloquent Magic - saving(), saved(), deleting(), etc on the model
These are built into laravel. We largely use them via Observers, and mostly for logging.
Interfaces/Contracts
(We do not currently use these)
🦾 Room for Improvement
This one is still being debated internally
Boot() Methods on the Model
We only use these in a few instances, but I think we could clean these up, as we're doing some presentation-style stuff in there that can likely just be (or is already) handled via accessors.
Actions
Newer addition - these (for now) handle making sure things can be deleted by checking to see if they have any "child" objects associated and perform the deletion. (The ability to delete suppliers for example because they don't have any associated FCOs.) These are only partially implemented currently. If we double-down on this approach, we'll move a lot of our isDeletable() logic in there, I suspect.
🦾 Room for Improvement
move a lot of our isDeletable() logic here?
lots more under internal discussion. I'm not 100% sold on this yet, but they (specifically @spencerrlongg) are trying to convince me.
Console
These are the guts behind our cli tooling, some of which get invoked via the GUI so we're not duplicating code (or because we need to run them in the background.)
🦾 Room for Improvement
We've been transitioning many of the older scripts to have nicer, table-based output where it makes sense. We should continue down that path. Nice to have, not mission critical.
Events
These are largely just constructors, but they hook into our notifications system.
🦾 Room for Improvement
Review these to see if the things they're doing should be there or somewhere else.
Exceptions
Prior to #17573 being merged, this directory mostly housed the default exception handler at app/Exceptions/Handler.php. This handler does kind of a lot, where we take what would 500 in other circumstances, we translate that into a human-readable error. Things like API requests that are invalid JSON, throws a validation or the request is unauthenticated, if a FCO isn't found, etc.
🦾 Room for Improvement
See if we can't clean up the default Exception handler
Possibly refactor some of our other validation logic to see if they make more sense in this directory
Helpers
As it says on the tin, these are static helper methods.
🦾 Room for Improvement
Revisit to see if we can solve some of these problems via the SnipeModel parent model accessors/mutators
Http\Controllers
Lots of good (if a little boring) work to be done here, making sure we're not duplicating logic, and making sure the shape of the API always uses the appropriate transformers.
Http\Requests
This handles all of our form requests (obviously), but something to note is that any controller (API or GUI) that allows for file image uploads will need to inherit from the ImageUploadRequest or the UploadFileRequest, which will validate the files to make sure they confirm with the file types we accept.
Http\Requests\Traits
Only contains one file - this should probably be moved or rethought.
Http\Requests\Transformers
These handle the "shape" of the API responses so that they are standardized. These are consumed by the API requests, and therefore are used by the table listings, since 99% of those are ajax calls.
🦾 Room for Improvement
I'm mostly pretty okay with these. They could probably use a revisit to make sure we're not repeating ourselves, and we could make a few more of these so that we can offer a full object in response or a compact version (for example, the way we handle created_by, we don't return the full user object, just the ID and the name. Including more would be a giant payload which wouldn't really be relevant for most use-cases.)
Http\Traits
These can probably ALL be moved into the model traits directory, or made into accessors/mutators.
Jobs
We don't currently use these. Possibly could be used if we end up implementing a queuing system (for notifications or imports, largely) but we'd need to fall back to just using mysql as the queue, since we don't want FOSS users to have to pay for RabbitMQ, SQS, etc.
Listeners
Mostly used for logging.
🦾 Room for Improvement
This feels like a lot of duplication, and we need a better strategy that's more consistent.
Livewire
This directory holds all of the Livewire controllers. We only use Livewire in a few areas, although that will likely change over time.
Mail
These sometimes handle mail-based notifications, envelope, etc. But sometimesNotifications handles that.
🦾 Room for Improvement
Make everything I just said not true. Modifying emails right now is a rictus of pain, because it's never exactly clear where local and envelopes are being set. These should exist in one place, either NotificationsorMail, but not both.
Models
These hold the model methods, model-level-validation, accessors, mutators, and query scopes for the FCOs.
🦾 Room for Improvement
Potentially move the query scopes into the more modern Laravel place for those to live. Not a big deal either way.
Models\Traits
We use these in a lot of places (notable FMCS, File Uploads and some validation rules)
Notifications
These handle whether or not to send an email, send a webhook, etc. We've had to do some weird stuff in here sometimes because of particular settings a user might have set up. This area needs a lot of work, and sometimes behaves a little inconsistently. Inconsistencies can include:
Email localization
Email envelope
Observers
These handle any special behaviors that should happen via the Eloquent magic. A lot of our logging happens here.
🦾 Room for Improvement
This feels like a lot of duplication, since we already do some logging in the boot() methods and also the listeners. Whichever way we decide to go (boot(), Listeners, Observers, or something else.
This is still being debated internally, but if @uberbrady gets his way, these will largely go away.
Providers
We use these quite a bit. They're good for behind-the-scenes processing, for example:
app/Providers/SnipeTranslationServiceProvider.php which handles normalizing the en to en-US (etc) locales
app/Providers/SamlServiceProvider.php which sets the SAML routes (why there and not the routes file, you might ask? No clue.)
app/Providers/ValidationServiceProvider.php which handles some of our custom validation. These should probably be moved into the Rules directory
Policies
We don't use these that much - mostly to set the table name for models where the model name and the table don't exactly match. Notably though:
app/Policies/SnipePermissionsPolicy.php handles the "generic" permissions on any FCO. blah.delete is covered here, and we don't have to add anything special when we add a new permission. If we add blah as a model, the standard permissions of create, edit, delete, etc will automagically be picked up.
🦾 Room for Improvement
We could probably lean a little harder on these in coordination with gates to simplify some things. That be dragons, for sure, but it might be worth the hassle in the long run.
Presenters
These largely handle the properties of the table lists. Whether or not a field should be shown/hidden by default, whether it's searchable, any special CSS classes, and so on. I'm mostly okay with these, but there's definitely some older methods in there that we're likely not using. We've been trending away from the $blah->present()->foo way of doing things, and will continue to work in that direction, using accessors when it makes sense - partially because if you end up with $item being null, you end up with a hard crash instead of a null value, and partially because the ->present()->foo stuff exists in the Presenters directory and also a few on the models themselves. 😬
🦾 Room for Improvement
Find every ->present()->foo and nuke it from orbit, instead preferring accessors.
Services
This needs looking at, since we already have Providers and this feels like it might be a little duplicative?
Rules
Laravel changed this behavior a while back, so we have some of our validation there, but also some validation on the models themselves (via Watson Validating for model-level validation.) We should definitely make a final decision on where this stuff should live.
Additional Notes:
Actions that should (optionally) trigger email/webhook notifications (and logs):
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
As Snipe-IT is a... let's say more mature project, it has obviously gathered some technical debt over the past decade and a half. New features got released in Laravel. Some good, some not so good, some worked well for us, and others we discovered weren't a great fit because of our own unique user base and feature set, and forward-looking goals.
Please note that this is not a place for feature requests - this is 100% code strategy. We already know what the feature requests are. As of this writing, we have over 950 open issues, so I promise that we know what folks are asking for.
Goals
Lexicon
These are terms you'll see used here since they are used within the organization as a shorthand. We won't be over-explaining Laravel built-ins, as they have their own documentation on that. 😀
This project is a lot more complicated than it looks on the face of things, largely due to admin-specific settings, things like FMCS, and the complex permission structure we provide.
Os Elefantes 🐘 🐘 🐘
There are a few elephants in the room that I want to get out of the way quickly.
We need to upgrade the AdminLTE theme (or pick another one) so that we're using a more current version of Bootstrap (or whatever other CSS framework we want to use. No, Tailwind is not currently being seriously considered at this time.)
A big part of our priorities here have always been (and must always be) to keep this project as simple to install as possible. Knowledge and access to a boring LAMP stack MUST be enough to get Snipe-IT running without subscribing to third party services, having special knowledge about queuing or caching systems, etc.
This does occasionally mean we can't add some bells and whistles, or if we do, we have to add them in such a way that they have sane fallbacks that still work without extra services.
We have traits in what feels like a million places. (It's not, it's like 4, but 4 feels like too many for what they do.) We need to have a word with ourselves and decide whether or how we can consolidate some of them so they're easier to find. We've already started some work on this, but there's more to be done.
Available Tooling
As I've been working on this, I realized a better approach is to go through all of our directories and give a quick overview of what were using, what we're not, and what functionality might be duplicated without us realizing it.
Eloquent Magic -
saving(),saved(),deleting(), etc on the modelThese are built into laravel. We largely use them via Observers, and mostly for logging.
Interfaces/Contracts
(We do not currently use these)
🦾 Room for Improvement
Boot()Methods on the ModelWe only use these in a few instances, but I think we could clean these up, as we're doing some presentation-style stuff in there that can likely just be (or is already) handled via accessors.
ActionsNewer addition - these (for now) handle making sure things can be deleted by checking to see if they have any "child" objects associated and perform the deletion. (The ability to delete suppliers for example because they don't have any associated FCOs.) These are only partially implemented currently. If we double-down on this approach, we'll move a lot of our
isDeletable()logic in there, I suspect.🦾 Room for Improvement
isDeletable()logic here?ConsoleThese are the guts behind our cli tooling, some of which get invoked via the GUI so we're not duplicating code (or because we need to run them in the background.)
🦾 Room for Improvement
EventsThese are largely just constructors, but they hook into our notifications system.
🦾 Room for Improvement
ExceptionsPrior to #17573 being merged, this directory mostly housed the default exception handler at
app/Exceptions/Handler.php. This handler does kind of a lot, where we take what would 500 in other circumstances, we translate that into a human-readable error. Things like API requests that are invalid JSON, throws a validation or the request is unauthenticated, if a FCO isn't found, etc.🦾 Room for Improvement
HelpersAs it says on the tin, these are static helper methods.
🦾 Room for Improvement
SnipeModelparent model accessors/mutatorsHttp\ControllersLots of good (if a little boring) work to be done here, making sure we're not duplicating logic, and making sure the shape of the API always uses the appropriate transformers.
Http\RequestsThis handles all of our form requests (obviously), but something to note is that any controller (API or GUI) that allows for file image uploads will need to inherit from the
ImageUploadRequestor theUploadFileRequest, which will validate the files to make sure they confirm with the file types we accept.Http\Requests\TraitsOnly contains one file - this should probably be moved or rethought.
Http\Requests\TransformersThese handle the "shape" of the API responses so that they are standardized. These are consumed by the API requests, and therefore are used by the table listings, since 99% of those are ajax calls.
🦾 Room for Improvement
I'm mostly pretty okay with these. They could probably use a revisit to make sure we're not repeating ourselves, and we could make a few more of these so that we can offer a full object in response or a compact version (for example, the way we handle
created_by, we don't return the full user object, just the ID and the name. Including more would be a giant payload which wouldn't really be relevant for most use-cases.)Http\TraitsThese can probably ALL be moved into the model traits directory, or made into accessors/mutators.
JobsWe don't currently use these. Possibly could be used if we end up implementing a queuing system (for notifications or imports, largely) but we'd need to fall back to just using mysql as the queue, since we don't want FOSS users to have to pay for RabbitMQ, SQS, etc.
ListenersMostly used for logging.
🦾 Room for Improvement
LivewireThis directory holds all of the Livewire controllers. We only use Livewire in a few areas, although that will likely change over time.
MailThese sometimes handle mail-based notifications, envelope, etc. But sometimes
Notificationshandles that.🦾 Room for Improvement
Make everything I just said not true. Modifying emails right now is a rictus of pain, because it's never exactly clear where local and envelopes are being set. These should exist in one place, either
NotificationsorMail, but not both.ModelsThese hold the model methods, model-level-validation, accessors, mutators, and query scopes for the FCOs.
🦾 Room for Improvement
Models\TraitsWe use these in a lot of places (notable FMCS, File Uploads and some validation rules)
NotificationsThese handle whether or not to send an email, send a webhook, etc. We've had to do some weird stuff in here sometimes because of particular settings a user might have set up. This area needs a lot of work, and sometimes behaves a little inconsistently. Inconsistencies can include:
ObserversThese handle any special behaviors that should happen via the Eloquent magic. A lot of our logging happens here.
🦾 Room for Improvement
boot()methods and also the listeners. Whichever way we decide to go (boot(), Listeners, Observers, or something else.ProvidersWe use these quite a bit. They're good for behind-the-scenes processing, for example:
app/Providers/SnipeTranslationServiceProvider.phpwhich handles normalizing theentoen-US(etc) localesapp/Providers/SamlServiceProvider.phpwhich sets the SAML routes (why there and not the routes file, you might ask? No clue.)app/Providers/ValidationServiceProvider.phpwhich handles some of our custom validation. These should probably be moved into theRulesdirectoryPoliciesWe don't use these that much - mostly to set the table name for models where the model name and the table don't exactly match. Notably though:
app/Policies/SnipePermissionsPolicy.phphandles the "generic" permissions on any FCO.blah.deleteis covered here, and we don't have to add anything special when we add a new permission. If we addblahas a model, the standard permissions ofcreate,edit,delete, etc will automagically be picked up.🦾 Room for Improvement
We could probably lean a little harder on these in coordination with gates to simplify some things. That be dragons, for sure, but it might be worth the hassle in the long run.
PresentersThese largely handle the properties of the table lists. Whether or not a field should be shown/hidden by default, whether it's searchable, any special CSS classes, and so on. I'm mostly okay with these, but there's definitely some older methods in there that we're likely not using. We've been trending away from the
$blah->present()->fooway of doing things, and will continue to work in that direction, using accessors when it makes sense - partially because if you end up with$itembeing null, you end up with a hard crash instead of a null value, and partially because the->present()->foostuff exists in thePresentersdirectory and also a few on the models themselves. 😬🦾 Room for Improvement
Find every
->present()->fooand nuke it from orbit, instead preferring accessors.ServicesThis needs looking at, since we already have Providers and this feels like it might be a little duplicative?
RulesLaravel changed this behavior a while back, so we have some of our validation there, but also some validation on the models themselves (via Watson Validating for model-level validation.) We should definitely make a final decision on where this stuff should live.
Additional Notes:
Actions that should (optionally) trigger email/webhook notifications (and logs):
Related Discussions/Issues
Beta Was this translation helpful? Give feedback.
All reactions