search

coderxio / dailymed-api

REST API for DailyMed SPLs
https://coderx.io/
MIT License
12 stars 5 forks source link

Yevgeny/scrapy refactor #11

Closed yevgenybulochnik closed 4 years ago

yevgenybulochnik commented 4 years ago

Fixes coderxio/dailymed-api #7 #6

Explanation

This is a very large PR with a lot of adjustments. Going forward I would really like to have smaller more frequent PRs. This PR refactors the scrapy items + adds a scrapy pipeline for loading data into the db. Because of the scrapy adjustments I also had to refactor the models and views. This PR does not include active or inactive ingredients at this time (xpaths are commented out currently)

@finish06 added some tests to the PR as well. I would like to make a proposal to use pytest vs djangos unittest.

Rationale

The scrapy pipeline is beneficial because it allows for better separation of concerns and makes data output processing more modular. The models and views needed to be adjusted to accommodate a Set > Spl > Product > Package setup, but these can all change. We will likely need many more serializers!

Tests

  1. What testing did you do?
    • Django unit tests run without issue
    • Loaded first 50 spls without issue
    • Browsable api shows correct views and responses (see attached spl output)
testing logs ``` Creating test database for alias 'default'... System check identified no issues (0 silenced). ... ---------------------------------------------------------------------- Ran 3 tests in 0.003s OK { "id": "fe1df76d-4925-444d-8e30-9dd07f205fb7", "products": [ { "id": 7, "packages": [ { "id": 12, "code": "68788-7665-3" }, { "id": 13, "code": "68788-7665-6" }, { "id": 14, "code": "68788-7665-9" }, { "id": 15, "code": "68788-7665-1" } ], "code": "68788-7665", "name": "Losartan Potassium" } ], "labeler": "Preferred Pharmaceuticals, Inc.", "set": "fe1df76d-4925-444d-8e30-9dd07f205fb7" } ```
yevgenybulochnik commented 4 years ago

Forgot to mention, I did not run code formatting on this branch. I would suggest we make a separate PR with all the code formatting updates, and make sure we stay consistent after that.

finish06 commented 4 years ago

I would agree the formatting changes should be a separate own branch. I used this branch last night when writing the test cases. I am comfortable with this merge.