TheGnarCo / GEAR

The Gnar Employee Asset Repository
0 stars 0 forks source link

Create Asset Model #20

Closed jrthreadgill closed 2 years ago

jrthreadgill commented 2 years ago

Create a data model for the application

jrthreadgill commented 2 years ago

@alxjrvs Here's what I was thinking for the data models:

erDiagram
  User ||--|{ Asset : has_many
  User {
  }
  Asset {
    id ID
    id user_id
    datetime created_at
    datetime updated_at
    string model_number
    string serial_number
    date approximate_purchase_date
    string mac_address
    string phone_number
    integer asset_type
  }

In this diagram, User specifies the User table generated by Devise without any added fields. Any user can have many assets.

Asset specifies the Asset table. Any given asset will belong to a single user. mac_address and phone_number will be nullable. asset_type will specify an integer, which will be mapped to a specific value (probably through an enum); this strategy was chosen in case we move this data to a database that does not support the ENUM field type.

Does all this make sense? Do you have any suggestions?

Tagging @efertsch on this as well. Thank you for helping me put this together!

jrthreadgill commented 2 years ago

@alxjrvs @stevemeisner After discussion with the mentors when I was doing GILD, it seemed like the best approach might be to make two separate tables: (1) asset_types and (2) assets. So then the asset_type column in the assets table would contain a reference to the asset_type table. Does that seem reasonable to you two or would you take a different tact?

alxjrvs commented 2 years ago

@jrthreadgill do you remember why this was suggests as opposed to an enum?

jrthreadgill commented 2 years ago

@alxjrvs We could definitely do that! If I remember right, there were just some strong feelings on the merits of integer vs. enum. Creating a separate table and a reference to that table seemed like something that most people could get behind. And that way we can just add rows to the asset_type table if/when we want to add a new asset type.

efertsch commented 2 years ago

@alxjrvs We could definitely do that! If I remember right, there were just some strong feelings on the merits of integer vs. enum. Creating a separate table and a reference to that table seemed like something that most people could get behind. And that way we can just add rows to the asset_type table if/when we want to add a new asset type.

I am not strongly opposed to this but it feels a bit like over-architecting/engineering to have a separate table instead of an enum given the size and intended use of this app but that's just my 🪙 🪙 😄

alxjrvs commented 2 years ago

I would second that, Ethan. I vote enum.

stevemeisner commented 2 years ago

My inclination would also have been to add an asset_type table. If this were for a client, I might contend for an asset_type table and full CRUD ops. The thinking being that the client could easily change their minds and add/edit types as the needs evolved.

With this being an internal tool for a software company and having a very limited number of conceivable "types" of gear, I'll plan to move forward with an enum for asset_type.

stevemeisner commented 2 years ago

Proposed initial asset type enum:

enum type: {
    laptop: 0,
    display: 1,
    keyboard: 2,
    mouse: 3,
    power_supply: 4,
    desk: 5,
    chair: 6,
    other: 7,
  }

I am thinking about adding an optional description field to the asset model. This could be particularly useful when we have an asset in "other". Thoughts?

efertsch commented 2 years ago

I am thinking about adding an optional description field to the asset model. This could be particularly useful when we have an asset in "other". Thoughts?

I wouldn't be opposed to that, and it could be a good exercise to implement a conditional validation on that i.e. description must be present if type is other.

I am curious what the "other" type might be though. It also seems like we want to be pretty specific with our cataloging (since it is inventory) so if it is not one of the first 6 items in this hash, should we add an enum value for it instead of lumping it in with "other"?

efertsch commented 2 years ago

Also, I think type might be a reserved keyword. I could be wrong about that but it might be worth checking out 😄

alxjrvs commented 2 years ago

It is - for polymorphic types (n o)

On Thu, Jul 21, 2022, 6:45 PM Ethan Fertsch @.***> wrote:

Also, I think type might be a reserved keyword. I could be wrong about that but it might be worth checking out 😄

— Reply to this email directly, view it on GitHub https://github.com/TheGnarCo/GEAR/issues/20#issuecomment-1192003948, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABKV2IYQBNT6NIJZD5FR4TVVHHGXANCNFSM5SZ5PX7Q . You are receiving this because you were mentioned.Message ID: @.***>

stevemeisner commented 2 years ago

for polymorphic types (n o)

@alxjrvs at first, I thought that you were making an old school emoji ʕʘ̅͜ʘ̅ʔ

I'm curious what that actually means :-)

stevemeisner commented 2 years ago

There is a notion of an "active" or "retired" asset. Should this be another enum on the Asset model?

efertsch commented 2 years ago

There is a notion of an "active" or "retired" asset. Should this be another enum on the Asset model?

Yes, maybe something like status?

alxjrvs commented 2 years ago

@efertsch @stevemeisner What about a retired_at timestamp?

alxjrvs commented 2 years ago

We could make a scope for active and retired keyed off of the presence of retired_at. Admittedly, enums do that for us as well, but timestamps do give us that extra bit of data.

alxjrvs commented 2 years ago

@stevemeisner Re: "Polymorphic" - I actually got my terms mixed up here. The Thing I was thinking of was "Single Table Inheritance", which is when you create several distinct Rails Models that all share a common database table, allowing you to inherit behaviors amongst ActiveRecord based models.

Check this out for more of what I mean!

efertsch commented 2 years ago

@efertsch @stevemeisner What about a retired_at timestamp?

I like that idea. Are there cases to account for where items are returned but not necessarily retired?

alxjrvs commented 2 years ago

I don't hate it, but we could keep it simple for now. Maybe "laptop" and "desk" for start? Or "laptop" and "misc"?

On Thu, Jul 21, 2022, 4:28 PM Steve Meisner @.***> wrote:

Proposed initial asset type enum:

enum type: { laptop: 0, display: 1, keyboard: 2, mouse: 3, power_supply: 4, desk: 5, chair: 6, other: 7, }

I am thinking about adding an optional description field to the asset model. This could be particularly useful when we have an asset in "other". Thoughts?

— Reply to this email directly, view it on GitHub https://github.com/TheGnarCo/GEAR/issues/20#issuecomment-1191902848, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABKV2MMCZ7D7HKT46MEQOLVVGXH5ANCNFSM5SZ5PX7Q . You are receiving this because you were mentioned.Message ID: @.***>

alxjrvs commented 2 years ago

I do like the description field, though!

And yeah, given that this is internal to a dev company, I think having it be an enum is almost more of a feature :p

If it ever becomes an issue, we can always make it more complicated later.

On Thu, Jul 21, 2022, 4:29 PM Alex Jarvis @.***> wrote:

I don't hate it, but we could keep it simple for now. Maybe "laptop" and "desk" for start? Or "laptop" and "misc"?

On Thu, Jul 21, 2022, 4:28 PM Steve Meisner @.***> wrote:

Proposed initial asset type enum:

enum type: { laptop: 0, display: 1, keyboard: 2, mouse: 3, power_supply: 4, desk: 5, chair: 6, other: 7, }

I am thinking about adding an optional description field to the asset model. This could be particularly useful when we have an asset in "other". Thoughts?

— Reply to this email directly, view it on GitHub https://github.com/TheGnarCo/GEAR/issues/20#issuecomment-1191902848, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABKV2MMCZ7D7HKT46MEQOLVVGXH5ANCNFSM5SZ5PX7Q . You are receiving this because you were mentioned.Message ID: @.***>