Skip to content
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

added in chicago elec dist #50

Merged

Conversation

bflatau
Copy link
Contributor

@bflatau bflatau commented Oct 18, 2022

Added in Chicago Elec. Dist.

id: "chicagoelecdistus",
name: "Chicago Elec. Dist.",
country: :us,
homepage: "https://chicagodist.com/"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove the trailing /. (Damn this OCD all the time :D)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry!

@PhillippOhlandt
Copy link
Member

The parsing looks good.

Two things:

  1. Could you change the vendor module name to ChicagoElectronicDistributorsUs (module and file names) but keep the shorter version for the vendor name field?

  2. Products that are in stock show a count on how many items are left, can you parse this as well? It's the same as for [VENDOR REQUEST]: Digi-Key (US) #30

Product that is on stock which has an items count: https://chicagodist.com/products/12-vdc-1000ma-regulated-switching-power-adapter-ul-listed

@bflatau
Copy link
Contributor Author

bflatau commented Oct 19, 2022

  1. I changed the names, I hope that's what you were after?
  2. I implemented a stock counter. I'm sure there's a better, more "elixir way" to do the syntax...
  • I may be missing a potential error state, I'm only checking for != [] at the moment. I figure if I'm calling Floki, it will always spit out a list vs giving nil or some other value/data?
  • I'm not sure if I picked the right HTML element, but I liked that I could grab a direct value vs. doing some sort of text-to-number parsing function for the innerText since it contains both the number we want plus some text.
  • Let me know what you think!

sku: "RPI4-MODBP-2GB"
},
%ProductUpdate{
url: "https://chicagodist.com/collections/raspberry-pi/products/raspberry-pi-4-model-b-1gb",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets change the url to https://chicagodist.com/products/raspberry-pi-4-model-b-1gb to be consistent with the other urls.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! I put them in order too (to help with the OCD ha)

Comment on lines 109 to 114
Floki.find(html_tree, ".product_form")
|> Floki.attribute("data-variant-inventory")
|> Enum.at(0)
|> Jason.decode()

get_in(elem(json_product_data, 1) |> Enum.at(0), ["inventory_quantity"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can change the code to this to reuse the existing variable and also simplify the last line where you get the value out of the json.

        available_quantity
        |> Enum.at(0)
        |> Jason.decode!()

      Enum.at(json_product_data, 0)["inventory_quantity"]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@PhillippOhlandt
Copy link
Member

Looks good to me! Thanks!
closes #29

@PhillippOhlandt PhillippOhlandt merged commit 51915da into nerves-metal-detector:master Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants