[BOT-161] Add md5sum to DGS reports

vostpt / bot

Source code of VOSTPTia - @vostpt's official bot

https://vostpt.github.io/bot/

MIT License

12 stars 4 forks source link

[BOT-161] Add md5sum to DGS reports #164

Closed joaoleveiga closed 4 years ago

joaoleveiga commented 4 years ago

Description

This pull request addresses issue BOT-161.

Task items:

[x] Fixed issue BOT-161;
[x] Added md5sum to reports based on their content;
[x] Added migration to create new md5sum field; ~Added migration to fill in this field if the database has previous reports;~
[x] Updated the documentation on how to run migrations;
[x] Ran eslint --fix ./src

joaoleveiga commented 4 years ago

A few notes:

If you prefer, the algorithm can be changed to sha256 instead of md5.
I still didn't do the migration script to update the checksums of reports alread present in the database. I'll try to figure that out tomorrow.

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 494

5 of 21 (23.81%) changed or added relevant lines in 3 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-0.4%) to 57.366%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/services/Corona.js	0	1	0.0%
src/api/Corona.js	4	19	21.05%
<!--	Total:	5	21	23.81%	-->

Files with Coverage Reduction	New Missed Lines	%
src/services/Corona.js	1	15.07%
<!--	Total:	1		-->

Totals
Change from base Build 488:	-0.4%
Covered Lines:	470
Relevant Lines:	763

💛 - Coveralls

miguelcarreiro commented 4 years ago

A few notes:

If you prefer, the algorithm can be changed to sha256 instead of md5. I don't have any preference in the algorithm, so, if you decided to use md5, I think there's no need to change algorithm to sha256.

I still didn't do the migration script to update the checksums of reports alread present in the database. I'll try to figure that out tomorrow. Ok, no problem! I'll wait for the script to merge PR into master.

joaoleveiga commented 4 years ago

@miguelcarreiro Whenever you can, can you please take a look at the migration file? I think I have probably tried 5 different approaches already and I still can't seem to understand how can I update the rows.

What's happening is that when I run the migration, it correctly fetches all the reports, and their md5sum, and tries to update. However, when committing the transaction (I think), only the last md5sum is used for all the rows :sob:

joaoleveiga commented 4 years ago

Updating the PR, as per our discussion on Discord about dropping the migration bit that updates the data.