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

feat(fetcher): implement vaksinasi.id db fetcher #806

Merged
merged 17 commits into from Oct 26, 2021
Merged

feat(fetcher): implement vaksinasi.id db fetcher #806

merged 17 commits into from Oct 26, 2021

Conversation

togetherwithasteria
Copy link
Contributor

@togetherwithasteria togetherwithasteria commented Oct 9, 2021

Fetches vaksinasi.id's DB and store it in wbw-vaccination-database.json

Closes #803

Description

The issue didn't give a lot of information, but I assume the goal is to have all the data from the API, as the endgoal is to write it all to wbw-vaccination-database. So this will fetch for the /regions endpoint of the vaksinasi.id API first, then iterate over the provinces and fetch its vaccination locations from the /locations/:region endpoint, dump all of the Promise'd JSON response into the promisedLocations array and then use Promise.all to await all of the promises in the locations array. Lastly, stringify locations and write it into wbw-vaccination-database.json

Just a quick question. I wrote the mockups for the responses in the vaksinid-${endpoint} format, but as I think this project uses the wbw prefix a lot, should I rename them to use the wbw prefix or not?

Also this is my first time contributing to big projects. Please forgive me for my mistakes.

Current Tasks

  • Typings (should I?)
  • Main code
  • Tests

Fetches vaksinasi.id's DB and store it in wbw-vaccination-database.json

Refs: #803
@netlify
Copy link

netlify bot commented Oct 9, 2021

✔️ Deploy Preview for wargabantuwarga ready!

🔨 Explore the source changes: d5de61b

🔍 Inspect the deploy log: https://app.netlify.com/sites/wargabantuwarga/deploys/616aa402ea93030007ec85f0

😎 Browse the preview: https://deploy-preview-806--wargabantuwarga.netlify.app

@codecov
Copy link

codecov bot commented Oct 9, 2021

Codecov Report

Merging #806 (d5de61b) into main (e373b9f) will increase coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #806      +/-   ##
==========================================
+ Coverage   86.92%   87.06%   +0.13%     
==========================================
  Files         133      134       +1     
  Lines        1423     1438      +15     
  Branches      454      455       +1     
==========================================
+ Hits         1237     1252      +15     
  Misses        181      181              
  Partials        5        5              
Impacted Files Coverage Δ
etc/fetchers/fetch-vaccination-database.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1487177...d5de61b. Read the comment docs.

@togetherwithasteria
Copy link
Contributor Author

Mm, should I put mark this as "Ready for Review" or not?

There is an ESLint warning in c84e78c, this commit addresses that warn
@togetherwithasteria togetherwithasteria marked this pull request as ready for review October 9, 2021 08:37
@zainfathoni
Copy link
Member

Just a quick question. I wrote the mockups for the responses in the vaksinid-${endpoint} format, but as I think this project uses the wbw prefix a lot, should I rename them to use the wbw prefix or not?

Since your vaksinid-${endpoint} is for your internal fetchers usage, I think it's okay to keep the vaksinid prefix.
We just used the wbw prefix because those data are provided by WBW, but in this case, the data is provided by another party, so I think putting the data source as the prefix makes sense.

Also this is my first time contributing to big projects. Forgive me for my n00b mistakes.

For a first-time contribution, your code seems sufficiently clean. You also write good tests as well. Thanks!

@togetherwithasteria
Copy link
Contributor Author

Also this is my first time contributing to big projects. Forgive me for my n00b mistakes.

For a first-time contribution, your code seems sufficiently clean. You also write good tests as well. Thanks!

Oh, well. Maybe I'm just too shy. Your welcome!

Copy link
Member

@zainfathoni zainfathoni left a comment

Choose a reason for hiding this comment

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

Overall, your implementation looks good and is going in the right direction.

However, we need you to perform further transformation into the data so that it's ready to be used downstream in the components.

etc/fetchers/fetch-vaccination-database.ts Show resolved Hide resolved
etc/fetchers/fetch-vaccination-database.ts Outdated Show resolved Hide resolved
etc/fetchers/fetch-vaccination-database.ts Outdated Show resolved Hide resolved
Copy link
Member

@mazipan mazipan left a comment

Choose a reason for hiding this comment

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

LGTM, wait for approval from others

@mazipan
Copy link
Member

mazipan commented Oct 10, 2021

You can contact me on Twitter @FortressValkyrie after this PR merged.

--> https://twitter.com/Maz_Ipan

etc/fetchers/fetch-wbw.ts Outdated Show resolved Hide resolved
etc/fetchers/fetch-vaccination-database.ts Outdated Show resolved Hide resolved
fetchVaccinationDatabase() is so fast that a spinner stucks the program.

So, move it to the top and remove line 37.
…tible

Transforms the output of the vaccination db fetcher to VaccinationContact[].

This will make it ready to be used downstream in the components.

The VaccinationContact interface has a 'map' property.
This is unecessary as WBW's "Buka Peta" button uses the address instead.
Though, I still add it in case we want to use that for the "Buka Peta" button.
…nce[]

Make it easy for the DB to be merged with the other contact DBs.

Using VaccinationContact[] might force us into unnecessarily iterating the array just to merge it.
Copy link
Contributor Author

@togetherwithasteria togetherwithasteria left a comment

Choose a reason for hiding this comment

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

Wait, will d4d8379 really help reducing unnecessary iterations? Idk, I'm dumb. If you want to revert this, just tell me.

@togetherwithasteria
Copy link
Contributor Author

Is ccb05a7 good?

@zainfathoni
Copy link
Member

I'll review it later tonight.

@zainfathoni
Copy link
Member

Wait, will d4d8379 really help reducing unnecessary iterations? Idk, I'm dumb. If you want to revert this, just tell me.

Yes, I think so. But I think it would be better if you can structure it into this format instead, so when we're iterating the provinces, we can easily get the data by accessing the property using computed values from the province name.
e.g. vaccinationDatabase[provinceName].

{
  "Aceh": [
    {
      "alamat": "Jalan Inong Balee No.38 Darussalam Banda Aceh",
      "lokasi": "Kota Banda Aceh",
      "buka_waktu": "08:00:00",
      "id": "0",
      "informasi_2": "-Senin dan Sabtu (bisa berubah sewaktu-waktu)\n-Dibeikan bagi warga dengan kriteria:\n1.petugas pelayanan publik\n2. Pra lansia &lansia\n3. Masyarakat Banda Aceh di utamakan yang berdomisili di wilayah Puskesmas Kopelma Darussalam,Rukoh,ie masen kayee adang,Lamgugob & Deah raya\n-Membawa KTP\n-informasi tambahan bisa dilihat di akun Instagram https://instagram.com/puskesmaskopelmadarussalam?utm_medium=copy_link",
      "keterangan": "Lokasi Vaksinasi COVID-19",
      "link": "https://www.instagram.com/p/CQgFAepnZkQ/?utm_medium=copy_link",
      "map": "https://goo.gl/maps/9hViESZJ9rYwGypb9",
      "mulai_tanggal": "2021-06-24",
      "penyedia": "UPTD PUSKESMAS KOPELMA DARUSSALAM",
      "rentang_umur": ["Dewasa (18-59 Tahun)", "Lansia (60-)"],
      "selesai_tanggal": "2022-03-31",
      "slug": "uptd-puskesmas-kopelma-darussalam",
      "terakhir_update": "2021-07-30",
      "tutup_waktu": "12:00:00",
      "verifikasi": 1
    }
  ]
}

Copy link
Member

@zainfathoni zainfathoni left a comment

Choose a reason for hiding this comment

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

Nice, thanks for making the changes! ✅

But I still have another minor request here, could you please address it? Thanks! 🙏

etc/fetchers/fetch-vaccination-database.ts Outdated Show resolved Hide resolved
@togetherwithasteria
Copy link
Contributor Author

togetherwithasteria commented Oct 12, 2021

f35f99b is done. Is this ready now?

Copy link
Member

@zainfathoni zainfathoni left a comment

Choose a reason for hiding this comment

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

The code seems good enough so far.

However, when I tried running it locally while logging each step of the API calls, it seems that it only managed to fetch the data for four provinces.

Screenshot 2021-10-12 at 22 14 44

Could you please try doing the same thing here in your local machine and share your results in screenshots? Thanks! 🙏

Screenshot 2021-10-12 at 22 19 29

Screenshot 2021-10-12 at 22 19 34

@togetherwithasteria
Copy link
Contributor Author

Could you please try doing the same thing here in your local machine and share your results in screenshots? Thanks! 🙏

Umm, seems negative.
image

image

@zainfathoni
Copy link
Member

Okay, then. Let's try seeing it in action at Netlify.
Please change mirror-box usage into fetch-wbw in this line and push your changes to this branch.

We will be able to see how it works in Netlify. If it breaks nothing, we can proceed with merging this PR. Thanks!

@togetherwithasteria
Copy link
Contributor Author

Wait, with the console.logs you put or not?

@zainfathoni
Copy link
Member

Wait, with the console.logs you put or not?

Without the console logging, we couldn't see anything here.

Screenshot 2021-10-14 at 20 39 07

Let's try adding the console logging and see how it goes.

@togetherwithasteria
Copy link
Contributor Author

togetherwithasteria commented Oct 15, 2021

6:05:09 PM: - Fetching next data...
6:05:09 PM: Aceh
6:05:09 PM: Sumatera Utara
6:05:09 PM: Sumatera Barat
6:05:09 PM: Riau
6:05:09 PM: Jambi
6:05:09 PM: Sumatera Selatan
6:05:09 PM: Bengkulu
6:05:09 PM: Lampung
6:05:09 PM: Kepulauan Bangka Belitung
6:05:09 PM: Kepulauan Riau
6:05:09 PM: DKI Jakarta
6:05:09 PM: Jawa Barat
6:05:09 PM: Jawa Tengah
6:05:09 PM: DI Yogyakarta
6:05:09 PM: Jawa Timur
6:05:09 PM: Banten
6:05:09 PM: Bali
6:05:09 PM: Nusa Tenggara Barat
6:05:09 PM: Nusa Tenggara Timur
6:05:09 PM: Kalimantan Barat
6:05:09 PM: Kalimantan Tengah
6:05:09 PM: Kalimantan Selatan
6:05:09 PM: Kalimantan Timur
6:05:09 PM: Kalimantan Utara
6:05:09 PM: Sulawesi Utara
6:05:09 PM: Sulawesi Tengah
6:05:09 PM: Sulawesi Selatan
6:05:09 PM: Sulawesi Tenggara
6:05:09 PM: Gorontalo
6:05:09 PM: Sulawesi Barat
6:05:09 PM: Maluku
6:05:09 PM: Maluku Utara
6:05:09 PM: Papua
6:05:09 PM: Papua Barat
6:05:12 PM: Aceh 186
6:05:12 PM: Sumatera Utara 1185
6:05:12 PM: Sumatera Barat 32
6:05:12 PM: Riau 67
6:05:32 PM: ✔ Fetching Database done in 23.768 seconds

Deployed it to a personal Netlify site. Yep, could reproduce your issue. Idk why though.

@togetherwithasteria
Copy link
Contributor Author

togetherwithasteria commented Oct 15, 2021

LET'S GET THIS MERGED!!!

9:08:29 PM: Papua Barat
9:08:47 PM: ✔ Fetching Database done in 18.907 seconds
9:08:47 PM: - Fetching next data...
9:09:53 PM: ✔ Fetching Sheets done in 84.806 seconds
9:09:53 PM: Kepulauan Bangka Belitung 26
9:09:53 PM: Lampung 18
9:09:53 PM: Riau 67
9:09:53 PM: DI Yogyakarta 41
9:09:53 PM: Sumatera Barat 32
9:09:53 PM: Jawa Timur 75
9:09:53 PM: Kepulauan Riau 85
9:09:53 PM: Nusa Tenggara Timur 5
9:09:53 PM: Kalimantan Utara 17
9:09:53 PM: Maluku 8
9:09:54 PM: Bengkulu 123
9:09:54 PM: Jawa Barat 128
9:09:54 PM: Sulawesi Tengah 43
9:09:54 PM: Maluku Utara 33
9:09:54 PM: Gorontalo 26
9:09:54 PM: Sulawesi Barat 32
9:09:54 PM: Papua 41
9:09:54 PM: Sulawesi Selatan 52
9:09:54 PM: Sulawesi Utara 17
9:09:54 PM: Kalimantan Tengah 17
9:09:54 PM: Banten 49
9:09:54 PM: Nusa Tenggara Barat 43
9:09:54 PM: Papua Barat 24
9:09:54 PM: Kalimantan Timur 28
9:09:54 PM: Aceh 186
9:09:54 PM: Kalimantan Barat 61
9:09:54 PM: DKI Jakarta 53
9:09:54 PM: Bali 86
9:09:54 PM: Sulawesi Tenggara 67
9:09:54 PM: Kalimantan Selatan 145
9:09:54 PM: Sumatera Selatan 330
9:09:54 PM: Jawa Tengah 370
9:09:55 PM: Sumatera Utara 1185
9:09:55 PM: $ next build
9:09:58 PM: info  - Using webpack 5. Reason: Enabled by default 

https://gist.github.com/FortressValkyrie/209da5c86d48e18f0bf825d01803a289

@togetherwithasteria
Copy link
Contributor Author

togetherwithasteria commented Oct 16, 2021

@zainfathoni

Is e94dbfb good yet? I didn't expect this PR to take so much time for this simple fetcher.

@togetherwithasteria
Copy link
Contributor Author

togetherwithasteria commented Oct 24, 2021

@zainfathoni Hey, I have fixed the bug already. Can you review it?
The fix is in e94dbfb.

@zainfathoni
Copy link
Member

Sorry for the slow response. Okay, I'll try to make some time later today to review it. Thanks! 🙏

@togetherwithasteria
Copy link
Contributor Author

Sorry for the slow response. Okay, I'll try to make some time later today to review it. Thanks! pray

Okay. Waiting for it.

Copy link
Member

@zainfathoni zainfathoni left a comment

Choose a reason for hiding this comment

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

LGTM 💯

Thanks for tirelessly working on this issue. Sorry for my slow response, I was pretty occupied in the past few weeks. 😅 🙏

@zainfathoni zainfathoni merged commit 93b1f76 into kawalcovid19:main Oct 26, 2021
@zainfathoni
Copy link
Member

@all-contributors please add @FortressValkyrie for code, test.

@allcontributors
Copy link
Contributor

@zainfathoni

I've put up a pull request to add @FortressValkyrie! 🎉

@zainfathoni zainfathoni added the hacktoberfest-accepted Accepted for Hacktoberfest participation label Oct 26, 2021
@togetherwithasteria
Copy link
Contributor Author

LGTM 100

Thanks for tirelessly working on this issue. Sorry for my slow response, I was pretty occupied in the past few weeks. sweat_smile pray

Your welcome. Haha, that's fine.

@mazipan
Copy link
Member

mazipan commented Oct 27, 2021

Can you contact me on the Twitter @FortressValkyrie ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted for Hacktoberfest participation
Projects
None yet
3 participants