wip-account-creation #4

Merged
lash merged 143 commits from wip-account-creation into master 2024-08-30 14:37:58 +02:00
Member

Created the initial menu using the go-vise engine

The current menu supports

  • User registration
  • PIN setup on registration
  • Send transaction flow with basic validation
  • Management of account details
  • PIN verification on guarded menu options
Created the initial menu using the go-vise engine The current menu supports - User registration - PIN setup on registration - Send transaction flow with basic validation - Management of account details - PIN verification on guarded menu options
Alfred-mk added 52 commits 2024-08-23 23:52:18 +02:00
Alfred-mk requested review from lash 2024-08-23 23:52:51 +02:00
carlos added 6 commits 2024-08-24 16:13:44 +02:00
carlos added 1 commit 2024-08-25 21:14:00 +02:00
carlos added 1 commit 2024-08-25 21:19:13 +02:00
lash requested changes 2024-08-26 04:56:50 +02:00
Dismissed
lash left a comment
Owner

This is good progress, guys.

There are quite a few comments here. Please start with the non-cosmetic ones (I'll make another pass and mark the priority ones).

When we have our next meeting, we can discuss how to approach the comments about DRYing/tidying.

This is good progress, guys. There are quite a few comments here. Please start with the non-cosmetic ones (I'll make another pass and mark the priority ones). When we have our next meeting, we can discuss how to approach the comments about DRYing/tidying.
@ -0,0 +21,4 @@
"git.defalsify.org/vise.git/state"
)
const (
Owner

Please add a documentation line on each.

Please add a documentation line on each.
Owner

priority

**priority**
lash marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +34,4 @@
USERFLAG_INVALID_AMOUNT
USERFLAG_QUERYPIN
USERFLAG_VALIDPIN
USERFLAG_INVALIDPIN
Owner

please rename this flag to USERFLAG_PINMISMATCH to avoid ambiguity.

please rename this flag to USERFLAG_PINMISMATCH to avoid ambiguity.
Owner

priority

**priority**
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +38,4 @@
USERFLAG_INCORRECTDATEFORMAT
)
const (
Owner

Please can we have all the http stuff in a separate package?

  • urls
  • responses
  • gets
Please can we have all the http stuff in a separate package? * urls * responses * gets
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +74,4 @@
} `json:"result"`
}
type fsData struct {
Owner

can this (and all its methods) be moved to a separate package please?

can this (and all its methods) be moved to a separate package please?
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +154,4 @@
func (fsd *fsData) save_yob(cxt context.Context, sym string, input []byte) (resource.Result, error) {
res := resource.Result{}
fp := fsd.path + "_data"
jsonData, err := os.ReadFile(fp)
Owner

This code is repeated many times. Could we please abstract the json fs part of the get and set to a module implementing an interface with Get and Put?

This code is repeated many times. Could we please abstract the json fs part of the get and set to a module implementing an interface with `Get` and `Put`?
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +165,4 @@
}
if len(input) > 0 {
yob := string(input)
accountData["YOB"] = yob
Owner

perhaps check this input, 2 or 4 digits, numeric

perhaps check this input, 2 or 4 digits, numeric
Owner

priority

**priority**
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +231,4 @@
case "2":
gender = "Female"
case "3":
gender = "Other"
Owner

is this a choice in the current ussd?

is this a choice in the current ussd?
Author
Member

Yes, the current USSD has the option of updating the gender, and the options are Male, Female and Other

Yes, the current USSD has the option of updating the gender, and the options are Male, Female and Other
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +278,4 @@
inputStr := string(input)
res := resource.Result{}
switch inputStr {
case "0":
Owner

this shouldnt switch on the input string; that requires manual syncing between here and the menu code.

how about using INCMP and the language switches on the sym instead?

Have a look at the lash/reuse-example go-vise branch, I've added an example that might illustrate.

this shouldnt switch on the input string; that requires manual syncing between here and the menu code. how about using INCMP and the language switches on the sym instead? Have a look at the `lash/reuse-example` go-vise branch, I've added an example that might illustrate.
Owner

priority

**priority**
Owner

I see this still isn't resolved.

I will make an issue of this, and mark it priority. We will handle it after merge.

I see this still isn't resolved. I will make an issue of this, and mark it priority. We will handle it after merge.
cmd/main.go Outdated
@ -0,0 +284,4 @@
case "1":
res.FlagSet = []uint32{state.FLAG_LANG}
res.Content = "swa"
default:
Owner

should error?

should error?
Owner

priority

**priority**
Author
Member

The switch only takes in two options, 0 or 1.
The menu remains on the select language node if an alternative input is provided

The switch only takes in two options, 0 or 1. The menu remains on the select language node if an alternative input is provided
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +708,4 @@
res.Content = amount
accountData["Amount"] = amount
Owner

Since the balance is already available, should check that input is not more than balance.

Since the balance is already available, should check that input is not more than balance.
Owner

priority

**priority**
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +762,4 @@
}
name := accountData["FirstName"]
gender := accountData["Gender"]
age := accountData["YOB"]
Owner

age needs to be calculated.

age needs to be calculated.
Owner

priority

**priority**
carlos marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +843,4 @@
return res, err
}
accountData["AccountPIN"] = accountPIN
Owner

The page says four digit PIN, so the input must be checked here.

The page says four digit PIN, so the input must be checked here.
Owner

priority

**priority**
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +858,4 @@
return res, nil
}
func (fsd *fsData) verify_pin(ctx context.Context, sym string, input []byte) (resource.Result, error) {
Owner

could clients of this reuse the pinentry used for unlock instead?

could clients of this reuse the pinentry used for `unlock` instead?
Author
Member

This sets a flag that shows the account has a PIN. Due to the difference in functionality I saw it best to use it instead of "unlock"

This sets a flag that shows the account has a PIN. Due to the difference in functionality I saw it best to use it instead of "unlock"
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +900,4 @@
fmt.Fprintf(os.Stderr, "starting session at symbol '%s' using resource dir: %s\n", root, dir)
ctx := context.Background()
st := state.NewState(15)
Owner

It's 14

It's 14
Alfred-mk marked this conversation as resolved
cmd/main.go Outdated
@ -0,0 +906,4 @@
state.FlagDebugger.Register(USERFLAG_ACCOUNT_CREATED, "ACCOUNT_CREATED")
state.FlagDebugger.Register(USERFLAG_ACCOUNT_SUCCESS, "ACCOUNT_SUCCESS")
state.FlagDebugger.Register(USERFLAG_ACCOUNT_PENDING, "ACCOUNT_PENDING")
state.FlagDebugger.Register(USERFLAG_INCORRECTPIN, "INCORRECTPIN")
Owner

Please complete this list

Please complete this list
Owner

priority

**priority**
carlos marked this conversation as resolved
@ -0,0 +1,5 @@
LOAD create_account 0
MOUT exit 0
Owner

This is not caught

This is not caught
Owner

priority

**priority**
Alfred-mk marked this conversation as resolved
@ -0,0 +2,4 @@
MOUT quit 9
HALT
INCMP confirm_create_pin 1
INCMP quit 9
Owner

if quit is chosen, next time the vm is started, the pin creation is never resumed, it just goes directly to main menu. It should prompt for setting pin again.

if quit is chosen, next time the vm is started, the pin creation is never resumed, it just goes directly to main menu. It should prompt for setting pin again.
Alfred-mk marked this conversation as resolved
@ -0,0 +2,4 @@
MOUT back 0
HALT
INCMP _ 0
INCMP select_gender *
Owner

See https://git.grassecon.net/urdt/ussd/pulls/4/files#issuecomment-1194

No matter where you start in the menu, you always go ahead to the end.

Again, if this is how current USSD behaves, we can keep this for now. But we should add a nice-to-have task to change that behavior to only edit full profile when not already edited.

See https://git.grassecon.net/urdt/ussd/pulls/4/files#issuecomment-1194 No matter where you start in the menu, you always go ahead to the end. Again, if this is how current USSD behaves, we can keep this for now. But we should add a nice-to-have task to change that behavior to only edit full profile when not already edited.
@ -0,0 +1,4 @@
MOUT back 0
HALT
INCMP _ 0
INCMP enter_familyname *
Owner

Is this how it works on the USSD now? This has you fill out the entire profile at once.

Is this how it works on the USSD now? This has you fill out the entire profile at once.
@ -0,0 +10,4 @@
INCMP send 1
INCMP vouchers 2
INCMP my_account 3
INCMP help 4
Owner

not implemented, and state gets stuck if its chosen. At a minimum please add an immediate quit in such cases.

not implemented, and state gets stuck if its chosen. At a minimum please add an immediate quit in such cases.
Alfred-mk marked this conversation as resolved
@ -0,0 +1,4 @@
LOAD unlock_account 0
HALT
RELOAD unlock_account
INCMP _ *
Owner

This can be MOVE _ ?

This can be `MOVE _` ?
carlos marked this conversation as resolved
@ -0,0 +7,4 @@
MOUT quit 9
HALT
INCMP _ 0
INCMP quit 9
Owner

This stops and gets stuck on correct pin put:

124125 will receive 22 from 0xA780F64fC93D8e78C20c4120f9E18AeEBeaD9631
Please enter your PIN to confirm:
0:Back
9:quit
1234
[I] engine:engine.go:173 new VM execution with input    input=1234
[D] engine:engine.go:182 start new VM run       code=0008015f0130000804717569740139
[T] vm:runner.go:81 new vm run
[D] vm:runner.go:113 execute code       opcode=8, op=INCMP, code=015f0130000804717569740139
[D] vm:runner.go:114 state=moves: 44 idx: 0 flags: 0x081b09 path: root/main/send/amount/transaction_pin lang: eng (English)
[T] vm:runner.go:361 testing sym        sym=_, input=31323334
[D] vm:runner.go:113 execute code       opcode=8, op=INCMP, code=04717569740139
[D] vm:runner.go:114 state=moves: 44 idx: 0 flags: 0x091b09 path: root/main/send/amount/transaction_pin lang: eng (English)
[T] vm:runner.go:361 testing sym        sym=quit, input=31323334
[T] vm:runner.go:200 no code remaining but not terminating
[D] vm:runner.go:113 execute code       opcode=6, op=MOVE, code=065f6361746368
[D] vm:runner.go:114 state=moves: 44 idx: 0 flags: 0x091b09 path: root/main/send/amount/transaction_pin lang: eng (English)
[T] engine:persist.go:47 that's a wrap  engine={0xc000140b00 0xc0000721a0}
[D] persist:fs.go:70 saved state and cache      key=aaaaa, bytecode=, flags=091b09
loop exited with error: unexpected termination: open /srv/git/grassrootseconomics/urdt-ussd.git/services/registration/_catch.bin: no such file or directory

exit status 1

This stops and gets stuck on correct pin put: ``` 124125 will receive 22 from 0xA780F64fC93D8e78C20c4120f9E18AeEBeaD9631 Please enter your PIN to confirm: 0:Back 9:quit 1234 [I] engine:engine.go:173 new VM execution with input input=1234 [D] engine:engine.go:182 start new VM run code=0008015f0130000804717569740139 [T] vm:runner.go:81 new vm run [D] vm:runner.go:113 execute code opcode=8, op=INCMP, code=015f0130000804717569740139 [D] vm:runner.go:114 state=moves: 44 idx: 0 flags: 0x081b09 path: root/main/send/amount/transaction_pin lang: eng (English) [T] vm:runner.go:361 testing sym sym=_, input=31323334 [D] vm:runner.go:113 execute code opcode=8, op=INCMP, code=04717569740139 [D] vm:runner.go:114 state=moves: 44 idx: 0 flags: 0x091b09 path: root/main/send/amount/transaction_pin lang: eng (English) [T] vm:runner.go:361 testing sym sym=quit, input=31323334 [T] vm:runner.go:200 no code remaining but not terminating [D] vm:runner.go:113 execute code opcode=6, op=MOVE, code=065f6361746368 [D] vm:runner.go:114 state=moves: 44 idx: 0 flags: 0x091b09 path: root/main/send/amount/transaction_pin lang: eng (English) [T] engine:persist.go:47 that's a wrap engine={0xc000140b00 0xc0000721a0} [D] persist:fs.go:70 saved state and cache key=aaaaa, bytecode=, flags=091b09 loop exited with error: unexpected termination: open /srv/git/grassrootseconomics/urdt-ussd.git/services/registration/_catch.bin: no such file or directory exit status 1 ```
Alfred-mk marked this conversation as resolved
carlos added 3 commits 2024-08-26 11:11:18 +02:00
carlos added 5 commits 2024-08-26 12:40:59 +02:00
Alfred-mk added 1 commit 2024-08-26 13:35:08 +02:00
Alfred-mk added 2 commits 2024-08-26 14:59:54 +02:00
carlos added 3 commits 2024-08-26 16:00:23 +02:00
carlos added 2 commits 2024-08-26 20:58:42 +02:00
carlos added 2 commits 2024-08-27 09:16:51 +02:00
carlos added 2 commits 2024-08-27 09:20:52 +02:00
Alfred-mk added 1 commit 2024-08-27 10:20:29 +02:00
carlos added 4 commits 2024-08-27 12:30:18 +02:00
Alfred-mk added 1 commit 2024-08-27 12:38:33 +02:00
carlos added 3 commits 2024-08-27 13:59:41 +02:00
Alfred-mk added 1 commit 2024-08-27 14:43:35 +02:00
Alfred-mk added 3 commits 2024-08-27 15:39:36 +02:00
carlos added 3 commits 2024-08-27 22:13:23 +02:00
Alfred-mk added 1 commit 2024-08-27 23:20:12 +02:00
carlos added 1 commit 2024-08-28 09:09:13 +02:00
carlos added 3 commits 2024-08-28 09:12:32 +02:00
Alfred-mk added 1 commit 2024-08-28 13:11:52 +02:00
carlos added 4 commits 2024-08-28 13:34:06 +02:00
Alfred-mk added 1 commit 2024-08-28 13:55:04 +02:00
Alfred-mk added 1 commit 2024-08-28 15:24:13 +02:00
carlos added 2 commits 2024-08-28 17:15:31 +02:00
Alfred-mk added 1 commit 2024-08-28 17:27:11 +02:00
carlos added 1 commit 2024-08-28 20:02:08 +02:00
lash requested changes 2024-08-29 16:18:02 +02:00
Dismissed
lash left a comment
Owner

A few small things to sort, then we are good.

A few small things to sort, then we are good.
@ -0,0 +75,4 @@
}
// if an account exists, set the flag and return
existingAccountData, err := h.accountFileHandler.ReadAccountData()
Owner

should not be necessary to set the flag when it already has been set and persisted. That will reduce one file access call each run.

should not be necessary to set the flag when it already has been set and persisted. That will reduce one file access call each run.
Alfred-mk marked this conversation as resolved
@ -0,0 +180,4 @@
// isValidPIN checks whether the given input is a 4 digit number
func isValidPIN(pin string) bool {
match, _ := regexp.MatchString(`^\d{4}$`, pin)
Owner

I prefer putting regexes like this on top of the file as a const.

I prefer putting regexes like this on top of the file as a const.
Alfred-mk marked this conversation as resolved
@ -0,0 +301,4 @@
case "2":
gender = "Female"
case "3":
gender = "Other"
Owner

Please change to "Unspecified"

Please change to "Unspecified"
@ -0,0 +341,4 @@
return res, nil
}
//ResetAccountUnlocked locks an account that had already been unlocked.
Owner

"unlocked" risks being an ambiguous term when we introduce account blocking due to direct action or incorrect pin attempts. can we rename it please?

"unlocked" risks being an ambiguous term when we introduce account blocking due to direct action or incorrect pin attempts. can we rename it please?
@ -0,0 +438,4 @@
res := resource.Result{}
switch codeFromCtx(ctx) {
case "swa":
res.Content = "Asante kwa kutumia huduma ya Sarafu. Kwaheri!"
Owner

were we using gettext here too?

were we using gettext here too?
Alfred-mk marked this conversation as resolved
Alfred-mk added 1 commit 2024-08-29 18:25:33 +02:00
carlos added 9 commits 2024-08-29 19:08:57 +02:00
carlos added 1 commit 2024-08-29 19:17:56 +02:00
Alfred-mk added 3 commits 2024-08-29 22:06:02 +02:00
carlos added 6 commits 2024-08-29 22:23:02 +02:00
carlos added 4 commits 2024-08-30 08:17:29 +02:00
Alfred-mk added 1 commit 2024-08-30 09:45:35 +02:00
carlos added 3 commits 2024-08-30 10:21:13 +02:00
Alfred-mk added 1 commit 2024-08-30 12:56:29 +02:00
carlos added 2 commits 2024-08-30 13:05:44 +02:00
lash approved these changes 2024-08-30 14:35:36 +02:00
lash merged commit ef0c207fc4 into master 2024-08-30 14:37:58 +02:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: urdt/ussd#4
No description provided.