ubiquibot / conversation-rewards

0 stars 10 forks source link

Save permits to DB #13

Closed rndquu closed 1 month ago

rndquu commented 4 months ago

Original comment

What should be done:

barebind commented 4 months ago

/start

ubiquibot[bot] commented 4 months ago

DeadlineThu, Mar 7, 6:46 AM UTC
Registered Wallet 0x91e6aF5A1E6a530d60949e1438036A4741B80D22
Tips:
ubiquibot[bot] commented 3 months ago
# These linked pull requests are closed:  <a href="https://github.com/ubiquibot/comment-incentives/pull/33">#33</a> 
gentlementlegen commented 2 months ago

@rndquu Following our Telegram conversation, I think we shall transfer this issue to https://github.com/ubiquibot/conversation-rewards if that is still relevant

rndquu commented 2 months ago

@gentlementlegen

There is this issue which adds a "Ubiquity virtual card" feature (by using gift cards API under the hood). Simply put, collaborators will have 2 options for withdrawing rewards: 1) Our usual permit URL 2) Payout to generated virtual card

The thing is that for this feature we don't need to save permits in a DB. We just need to save/maintain "debits and credits" (in terms of pavlovcik's supabase DB instance) ledger table and only save smth to permits or virtual cards tables when collaborators actually want to withdraw a reward.

So I wanted to modify the current issue to implement the "ledger style" DB reward table. What do you think?

Screenshot 2024-04-20 at 00 35 06
gentlementlegen commented 2 months ago

@rndquu makes sense to me. In terms of DB structure, credits and debits would become one table (maybe transactions) and it would keep a record of any financial flow that occurred for a user correct?

But is it solving the fact that we do not save permits to the DB or is it something that would be aside this task?

rndquu commented 2 months ago

In terms of DB structure, credits and debits would become one table (maybe transactions) and it would keep a record of any financial flow that occurred for a user correct?

The transactions table is, as far as I understand, the settlements table in terms of pavlovcik's DB instance

Screenshot 2024-04-21 at 00 26 37

But is it solving the fact that we do not save permits to the DB or is it something that would be aside this task?

From my understanding, when the transactions/settlements table is ready the permits table (along with the virtual_cards table) will be updated only when user requests a withdraw either in crypto (the way we use it now) either to a virtual card.

gentlementlegen commented 2 months ago

All these tables are empty so far so pretty safe to update them. But in the end, we should still push the generated permits to the table then, which is not yet the case right now. We might need a separate task for this?

0x4007 commented 2 months ago

For additional context on why I broke everything apart into so many tables: I intended for transactions to also accommodate other forms of "currency" like XP

Permits correspond only to "cash" (crypto) rewards.

For example a project admin can, for example, arbitrarily add 100 to your XP. So now your repo XP is registered as a "credit" (with no payment permit associated) with a value of 100.

The original intent was that I wanted to couple the XP scoring with dollars earned in the system. And then add additional modifiers on top primarily for subtractions for poor performance (without penalizing money)

Maybe there could be a more elegant way to handle matters but I figured if they are so tightly coupled it would make sense to leave as is.

gentlementlegen commented 2 months ago

I don't think credit and debit should be separate tables. I don't know if XP should be on its own, this can be figured out when we implement it. Probably has to if we had Tiers and Rewards later on. Because in the end credit is a positive value and debit a negative value. It is way easier to count / sort them if they exist on the same table, and it would be a common use case to have to fetch all of them ordered by date which implies lots of cross joins if they are separated. On the other hand, Postgres allows for Views to solve that as well. As long as the tables are properly indexed the performance should be similar.

gentlementlegen commented 2 months ago

/start

ubiquibot[bot] commented 2 months ago

Warning! This task was created over 59 days ago. Please confirm that this issue specification is accurate before starting.
DeadlineSun, May 5, 8:57 AM UTC
Registered Wallet 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED
Tips:
ubiquibot[bot] commented 1 month ago
+ Evaluating results. Please wait...
ubiquibot[bot] commented 1 month ago

[ 12.9 WXDAI ]

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment112.9
Conversation Incentives
CommentFormattingRelevanceReward
For additional context on why I broke everything apart into so m...
12.90.7412.9

[ 53.3 WXDAI ]

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueTask125
IssueComment40
IssueComment428.3
Conversation Incentives
CommentFormattingRelevanceReward
@rndquu Following our Telegram conversation, I think we shall tr...
-0.15-
@rndquu makes sense to me. In terms of DB structure, `credits` a...
-
code:
  count: 3
  score: "0"
  words: 3
0.805-
All these tables are empty so far so pretty safe to update them....
-0.82-
I don't think credit and debit should be separate tables. I don'...
-0.765-
@rndquu Following our Telegram conversation, I think we shall tr...
2.40.152.4
@rndquu makes sense to me. In terms of DB structure, `credits` a...
9
code:
  count: 3
  score: "3"
  words: 3
0.8059
All these tables are empty so far so pretty safe to update them....
4.40.824.4
I don't think credit and debit should be separate tables. I don'...
12.50.76512.5

[ 1.7 WXDAI ]

@gitcoindev
Contributions Overview
ViewContributionCountReward
ReviewComment11.7
Conversation Incentives
CommentFormattingRelevanceReward
I also did a review. The error handling looks good, after tests ...
1.70.221.7

[ 63 WXDAI ]

@rndquu
Contributions Overview
ViewContributionCountReward
IssueSpecification19
IssueComment254
Conversation Incentives
CommentFormattingRelevanceReward
Original [comment](https://github.com/ubiquity/pay.ubq.fi/issues...
9
a:
  count: 2
  score: "2"
  words: 2
li:
  count: 1
  score: "1"
  words: 35
code:
  count: 1
  score: "1"
  words: 1
19
@gentlementlegen There is [this](https://github.com/ubiquity...
31.8
a:
  count: 2
  score: "2"
  words: 4
li:
  count: 2
  score: "2"
  words: 9
0.73531.8
> In terms of DB structure, credits and debits would become one ...
22.2
code:
  count: 5
  score: "5"
  words: 6
0.822.2
0x4007 commented 1 month ago

I don't think credit and debit should be separate tables.

It's impossible to have a permit associated with a debit so it doesn't make sense to combine them. Then you have wasted cells.