GoodDollar / GoodServer

Backend to support the GoodDAPP
MIT License
13 stars 12 forks source link

Bug filter null torus providers #372

Closed roma-hladilka closed 2 years ago

roma-hladilka commented 2 years ago

Description

Checklist:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging 855277b9e7586f72df317708c91e798a17e863de into f0c983273a5218509f13cd24e85da8656381cfd7 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 1 alert when merging e6724bf551308dd15de110ff66b26da067c81c51 into f0c983273a5218509f13cd24e85da8656381cfd7 - view on LGTM.com

fixed alerts:

sirpy commented 2 years ago

@johnsmith-gooddollar how do we know that we can delete those records? maybe there are some older records before we had the field torusProvider? you need some more strict indicator for an abandoned signup record

johnsmith-gooddollar commented 2 years ago

@sirpy @roma-hladilka yes, i also thought about it. Looking for more exact indicators now

johnsmith-gooddollar commented 2 years ago

Brief comparison

FINE

image

BROKEN

image
johnsmith-gooddollar commented 2 years ago

@roma-hladilka i suggest to change conditions

sirpy commented 2 years ago

@serdiukov-o-nordwhale basically we have the isCompleted field that should give an indication if signup process was done also if lastLogin is relatively newer than createdDate that might also give an indication of an active account isVerified is and indication for an active account

lgtm-com[bot] commented 2 years ago

This pull request fixes 2 alerts when merging eb8f113a0c7e429998d993c8c461c61ae117eb19 into 61444e9ae9687bef3b8d6a783a89a222468571c7 - view on LGTM.com

fixed alerts:

sirpy commented 2 years ago

@roma-hladilka @johnsmith-gooddollar w3token and logintoken are not relevant anymore

sirpy commented 2 years ago

@johnsmith-gooddollar before actually removing abandoned records, please run just the query and send the results

johnsmith-gooddollar commented 2 years ago

@roma-hladilka ^^^

lgtm-com[bot] commented 2 years ago

This pull request fixes 2 alerts when merging 65f61fb6dd3c3b2b671cd41f7aaf92b3ca725712 into 61444e9ae9687bef3b8d6a783a89a222468571c7 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 2 alerts when merging 466a8d3f6824ddcdd32efb634f30dc46ce0fe5ab into 61444e9ae9687bef3b8d6a783a89a222468571c7 - view on LGTM.com

fixed alerts:

johnsmith-gooddollar commented 2 years ago

@johnsmith-gooddollar before actually removing abandoned records, please run just the query and send the results

@roma-hladilka ^^^^

lgtm-com[bot] commented 2 years ago

This pull request fixes 2 alerts when merging 309b8bbbed8d6e5934cb90f25f5f501bac8b3fd0 into 0f9d30d631066ed216d84172cd8d1b9ecb6cc801 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 2 years ago

This pull request fixes 2 alerts when merging 062b3f48a4ef4c5baf9ba782668dd214dd32967d into 0f9d30d631066ed216d84172cd8d1b9ecb6cc801 - view on LGTM.com

fixed alerts:

roma-hladilka commented 2 years ago

I removed lastLogin and createdDate comparing, as there is a chance that users was not login several times, so lastLogin will be less than createdDate. Now query will remove about 45K accounts, most of which don't have any user info (registration wasn't completed) and some of them miss lastLogin field (some old 2020 accounts, which didn't re login for a long time)

johnsmith-gooddollar commented 2 years ago

@roma-hladilka please attach those 45k records. export them in any way (json, also you could attach an .xlsx sheet)

sirpy commented 2 years ago

@roma-hladilka @johnsmith-gooddollar this is public, dont post those things here

johnsmith-gooddollar commented 2 years ago

@roma-hladilka make spreadsheet, upload it onto GoodDollar's GoogleDrive and share with me & @sirpy

johnsmith-gooddollar commented 2 years ago

@sirpy

  1. I've added env var to enable/disable cron task. It's disabled by default, so i could merge PR now
  2. Have you checked the spreadsheet we prepared ? Are the records selected ok to be removed ? Please notify us once you'll check.
  3. After you'll check we could make some decision - to enable task or to add some suggestions to the search query