Debugging usercredit, userprofile, balance display issues, Kenyon #346

pwdel · 2024-09-27T13:38:43Z

Attempting to debug usercredit problem seen on Kenyon instance

pwdel · 2024-09-27T16:53:40Z

Tested on local after modifications, it's displaying credits as expected in the case of no wins, only investment so far, from one user, as previous. This is after having converted the integertype to int64.

Need to possibly look at building scenarios that mimic the Kenyon betting history.
Need to write tests for the balance display as this also seems to be a known problem.

pwdel · 2024-09-27T17:00:45Z

Setup we're working with for test case:

https://github.com/ajlacey/socialpredict/blob/main/backend/setup/setup.yaml

economics:
  marketcreation:
    initialMarketProbability: 0.5
    initialMarketSubsidization: 10
    initialMarketYes: 0
    initialMarketNo: 0
  marketincentives:
    createMarketCost: 10
    traderBonus: 1
  user:
    initialAccountBalance: 0
    maximumDebtAllowed: 5000
  betting:
    minimumBet: 1
    betFees:
      initialBetFee: 1
      eachBetFee: 0
      sellSharesFee: 0

… in publicuser package.

ajlacey · 2024-09-29T02:57:46Z

backend/server/server.go

 	// user portfolio, (which is public)
-	router.HandleFunc("/v0/portfolio/{username}", usershandlers.GetPublicUserPortfolio).Methods("GET")
+	router.HandleFunc("/v0/portfolio/{username}", publicuser.GetPublicUserPortfolio).Methods("GET")


because we use the package name before calling the handler, this call looks redundant (it stutters). Consider renaming the function to Portfolio or GetPortfolio so that the call would look like publicuser.Portfolio or publicuser.GetPortfolio.

This can be applied to the other handlers that are being updated in the server as well.

ajlacey · 2024-09-29T03:00:25Z

backend/handlers/users/publicuser/publicuserportfolio.go

consider renaming this file to portfolio.go since it is a member of the publicuser package.

ajlacey · 2024-09-29T03:01:54Z

backend/handlers/users/publicuser/publicuserportfolio_test.go

No need to add an empty test file. If you're planning to add tests, I would add this file as part of the PR that adds the tests.

ajlacey · 2024-09-29T03:04:24Z

backend/handlers/users/publicuser/publicuser_test.go

+	// Migrate the User model
+	if err := db.AutoMigrate(&models.User{}); err != nil {
+		t.Fatalf("Failed to migrate user model: %v", err)
+	}


NewFakeDB(t) migrates the models on creation, so this is redundant.

ajlacey · 2024-09-29T03:07:29Z

backend/handlers/users/privateuserinfo/privateuser_test.go

same as publicuser_test- this isn't necessary unless you're adding tests. Since there is no new privateuser code here, I think it makes sense to add this when we create tests for the privateuser package.

ajlacey · 2024-09-29T03:08:58Z

backend/handlers/users/credit/usercredit_test.go

@@ -0,0 +1,68 @@
+package usercredit_test


I don't think this should be a separate package. We're not exporting test functionality here.

ajlacey · 2024-09-29T03:09:27Z

backend/handlers/users/credit/usercredit.go

@@ -53,12 +55,11 @@ func GetUserCreditHandler(w http.ResponseWriter, r *http.Request) {

 }

-func calculateUserCredit(db *gorm.DB, username string) int {
+func CalculateUserCredit(db *gorm.DB, username string, maximumdebt int64) int64 {


This function can remain private when the testing is done in the same package.

ajlacey · 2024-09-29T03:10:08Z

backend/handlers/users/credit/usercredit_test.go

+	db := modelstesting.NewFakeDB(t)
+	if err := db.AutoMigrate(&models.User{}); err != nil {
+		t.Fatalf("Failed to migrate models: %v", err)
+	}


db migration happens when creating the fake.

ajlacey · 2024-09-29T03:13:24Z

backend/handlers/users/credit/usercredit_test.go

+			Username:       test.username,
+			Email:          test.email,
+			AccountBalance: test.accountBalance,
+			ApiKey:         test.apiKey,


Is this field necessary to create a user? It doesn't look like it, but I may misunderstand the gorm struct tagging. If it's not necessary, I don't think we're using this field in the tests at all and can safely remove it.

Adding usercredit and test.

b99985b

pwdel requested a review from ajlacey September 27, 2024 13:38

pwdel self-assigned this Sep 27, 2024

pwdel linked an issue Sep 27, 2024 that may be closed by this pull request

Credits Not Displaying Properly Under Certain Conditions #325

Open

Updating, having failed to update server previously.

a1e2cac

Updating, including package update for publicuserportfolio, including…

6f172fa

… in publicuser package.

pwdel changed the title ~~Adding usercredit and test.~~ Debugging usercredit, userprofile, balance display issues, Kenyon Sep 27, 2024

Uscredit refactored to accept maximum debt as an input.

c0387c9

ajlacey requested changes Sep 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Debugging usercredit, userprofile, balance display issues, Kenyon #346

Debugging usercredit, userprofile, balance display issues, Kenyon #346

pwdel commented Sep 27, 2024

pwdel commented Sep 27, 2024

pwdel commented Sep 27, 2024

ajlacey Sep 29, 2024

ajlacey Sep 29, 2024

ajlacey Sep 29, 2024

ajlacey Sep 29, 2024

ajlacey Sep 29, 2024

ajlacey Sep 29, 2024

ajlacey Sep 29, 2024

ajlacey Sep 29, 2024

ajlacey Sep 29, 2024

Debugging usercredit, userprofile, balance display issues, Kenyon #346

Are you sure you want to change the base?

Debugging usercredit, userprofile, balance display issues, Kenyon #346

Conversation

pwdel commented Sep 27, 2024

pwdel commented Sep 27, 2024

pwdel commented Sep 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment