wuwanahq / wuwana2

🇪🇺 Wuwana v2 - A webapp to help users find local suppliers in Spain.
Mozilla Public License 2.0
2 stars 2 forks source link

Scraper set no ProvinceID when location is unknown #180

Open Nils85 opened 3 years ago

Nils85 commented 3 years ago

@kakaye-mkubwa Thank you for the PR, few things:

1. Could we show the company with the ID 'ES' for Spain when we did not detect a postal code?

2. For the postal code: instead of assigning 1 to ES-VI, can we do 01 to ES-VI, instead? In ES-VI, their postal code starts with 01 (postal code example: 01001)

This is from me. Perhaps @Nils85 has more comments.

As for (1) the way we can sort it out is by creating a record for holding data in the case we did not find a postal code. Since just assigning ES will lead to issues with foreign key checks.

As for (2) lemme change the structure of the tsv. As for the table structure, I have set the postal code column to varchar so as to support values with leading zeros.

Made a commit that sorts out the second (2) concern that was raised by @levogirar and a fix for the first one (1), since 'ES' is displayed but not stored on the database due to foreign key checks.

Originally posted by @kakaye-mkubwa in https://github.com/wuwanahq/wuwana2/issues/144#issuecomment-787012198

Nils85 commented 3 years ago

The default value should be "ES" (for Spain) when a company location is unknown. But currently the column ProvinceID is empty in the Company table when this situation happens. And anyway there is no ProvinceID = ES in the table Province...

See: Scraper::mergeCompanyData DataAccess\Company::update DataAccess\Company::fetchProvinceID

kakaye-mkubwa commented 3 years ago

This is how I did it, the provinceID column remains empty. When displaying the company details in the UI, 'ES' is displayed if the provinceID is empty, otherwise, the provinceID value is displayed. Why I left it empty? If I add a 'ES' column in the Province table, I will need to add a corresponding column in the region table as well.

Nils85 commented 3 years ago

If I add a 'ES' column in the Province table, I will need to add a corresponding column in the region table as well.

Yes exactly, this solution makes more sense! It is simpler and like that we can translate the country name. The question is, why didn't you do that? Instead of replacing the object property at the very end, in the view...

kakaye-mkubwa commented 3 years ago

Here is the proposed change to the location information tables, take note of the final records in each table. Check it @Nils85 , then tell me whether to proceed making the change. Updated Google Sheet

Nils85 commented 3 years ago

It is not needed to add a new line for Spain (code 53) in the table PostalCode.