wip-account-creation #4
			No reviewers
			
		
		
		
	
	
	
		Labels
		
	
	
	
	
		No Label
		
			
	
	
	Compat/Breaking
		
			Kind/Bug
		
			Kind/Documentation
		
			Kind/Enhancement
		
			Kind/Feature
		
			Kind/Security
		
			Kind/Testing
		
			Priority
Critical
		
			Priority
High
		
			Priority
Low
		
			Priority
Medium
		
			Reviewed
Confirmed
		
			Reviewed
Duplicate
		
			Reviewed
Invalid
		
			Reviewed
Won't Fix
		
			Status
Abandoned
		
			Status
Blocked
		
			Status
Need More Info
		
		
			Activity
Doing
		
			Activity
Hold
		
			Activity
Proposal
		
			Activity
QA
		
			Activity
Validate
		
			Runner
AT
		
			Runner
CLI
		
			Runner
HTTP
		
			Runner
SSH
		
			cleanup
		
			devops
		
			documentation
		
			easypeasy
		
			exchange
		
			i18n
		
			legacy
		
			meta
		
			migration
		
			optimization
		
			privilege
		
			refactor
		
			smell
		
			support
		
			tooling
		
			ux
		
	
		No Milestone
		
			
		
	
	
		
		
		
			No project
			
				
			
		
	
	
	
	
	
		No Assignees
		
			
		
	
	
	
		3 Participants
		
	
	
		
		
			Notifications
			
				
			
		
	
	
		
		
	
	
	Due Date
	No due date set.
			
				Dependencies
				
				
		
	
	
	No dependencies set.
			Reference: urdt/ussd#4
			
		
	
		Loading…
	
		Reference in New Issue
	
	Block a user
	
	No description provided.
		
		Delete Branch "wip-account-creation"
	
	Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Created the initial menu using the go-vise engine
The current menu supports
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 (Please add a documentation line on each.
priority
@ -0,0 +34,4 @@USERFLAG_INVALID_AMOUNTUSERFLAG_QUERYPINUSERFLAG_VALIDPINUSERFLAG_INVALIDPINplease rename this flag to USERFLAG_PINMISMATCH to avoid ambiguity.
priority
@ -0,0 +38,4 @@USERFLAG_INCORRECTDATEFORMAT)const (Please can we have all the http stuff in a separate package?
@ -0,0 +74,4 @@} `json:"result"`}type fsData struct {can this (and all its methods) be moved to a separate package please?
@ -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)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
GetandPut?@ -0,0 +165,4 @@}if len(input) > 0 {yob := string(input)accountData["YOB"] = yobperhaps check this input, 2 or 4 digits, numeric
priority
@ -0,0 +231,4 @@case "2":gender = "Female"case "3":gender = "Other"is this a choice in the current ussd?
Yes, the current USSD has the option of updating the gender, and the options are Male, Female and Other
@ -0,0 +278,4 @@inputStr := string(input)res := resource.Result{}switch inputStr {case "0":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-examplego-vise branch, I've added an example that might illustrate.priority
I see this still isn't resolved.
I will make an issue of this, and mark it priority. We will handle it after merge.
@ -0,0 +284,4 @@case "1":res.FlagSet = []uint32{state.FLAG_LANG}res.Content = "swa"default:should error?
priority
The switch only takes in two options, 0 or 1.
The menu remains on the select language node if an alternative input is provided
@ -0,0 +708,4 @@res.Content = amountaccountData["Amount"] = amountSince the balance is already available, should check that input is not more than balance.
priority
@ -0,0 +762,4 @@}name := accountData["FirstName"]gender := accountData["Gender"]age := accountData["YOB"]age needs to be calculated.
priority
@ -0,0 +843,4 @@return res, err}accountData["AccountPIN"] = accountPINThe page says four digit PIN, so the input must be checked here.
priority
@ -0,0 +858,4 @@return res, nil}func (fsd *fsData) verify_pin(ctx context.Context, sym string, input []byte) (resource.Result, error) {could clients of this reuse the pinentry used for
unlockinstead?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"
@ -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)It's 14
@ -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")Please complete this list
priority
@ -0,0 +1,5 @@LOAD create_account 0MOUT exit 0This is not caught
priority
@ -0,0 +2,4 @@MOUT quit 9HALTINCMP confirm_create_pin 1INCMP quit 9if 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.
@ -0,0 +2,4 @@MOUT back 0HALTINCMP _ 0INCMP select_gender *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 0HALTINCMP _ 0INCMP enter_familyname *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 1INCMP vouchers 2INCMP my_account 3INCMP help 4not implemented, and state gets stuck if its chosen. At a minimum please add an immediate quit in such cases.
@ -0,0 +1,4 @@LOAD unlock_account 0HALTRELOAD unlock_accountINCMP _ *This can be
MOVE _?@ -0,0 +7,4 @@MOUT quit 9HALTINCMP _ 0INCMP quit 9This stops and gets stuck on correct pin put:
A few small things to sort, then we are good.
@ -0,0 +75,4 @@}// if an account exists, set the flag and returnexistingAccountData, err := h.accountFileHandler.ReadAccountData()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.
@ -0,0 +180,4 @@// isValidPIN checks whether the given input is a 4 digit numberfunc isValidPIN(pin string) bool {match, _ := regexp.MatchString(`^\d{4}$`, pin)I prefer putting regexes like this on top of the file as a const.
@ -0,0 +301,4 @@case "2":gender = "Female"case "3":gender = "Other"Please change to "Unspecified"
@ -0,0 +341,4 @@return res, nil}//ResetAccountUnlocked locks an account that had already been unlocked."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!"were we using gettext here too?