feat: Added support for memo

[AbhayAysola]

Closes #88
Account Number: 8928526805de48e4bf2ed2a9b4b839e6b2603018ecdfbf4cffdb2065e01a3ed1

[AbhayAysola]

@zinoadidi This should work as far as I know, but since I can't test it anywhere it might not work as expected.

[zinoadidi]

will you update when you manage to test it;
also, tests should be updated as well to reflect possibility with memo
and docs
and I saw some places where 'empty string' was used instead of STRING type didn't pay much attention but could be something to check in code

[AbhayAysola]

will you update when you manage to test it;
also, tests should be updated as well to reflect possibility with memo
and docs
and I saw some places where 'empty string' was used instead of STRING type didn't pay much attention but could be something to check in code

Sure, I will update docs and tests when I can confirm that it works.
The empty string is done on purpose since memo isn't mandatory.

[zinoadidi]

ok cool;
Maybe something to check, I remember with TS it doesn't include the field if its not set
as I read in the discord chat, bucky says its possible to omit the field from request if user doesn't provide it.

Its just a theory, we can check it

[AbhayAysola]

Yeah that shouldn't be an issue we can test it when the banks are up.

[tomijaga]

Hey @AbhayAysola there is already a PR on the account manager for the memo.
I think you can use this to implement the memo here since it was made by the core team

[tomijaga]

Here's the PR thenewboston-blockchain/Account-Manager#615

[AbhayAysola]

Here's the PR thenewboston-developers/Account-Manager#615

Yep I used that

[tomijaga]

@AbhayAysola Okay so all that's left is:

• check that the memo only has alphanumeric characters by using the
  same regex expression as in the PR: /^[a-zA-Z0-9_ ]*$/

• Remove leading spaces using the .trim() method

• And if memo is empty just remove it from the transaction object

Do all of these in the createTransaction() method in the PaymentHandler class

This is the last thing needed for the migration to the account manager. I could make the updates on your branch if you want

[AbhayAysola]

@AbhayAysola Okay so all that's left is:

• check that the memo only has alphanumeric characters by using the
  same regex expression as in the PR: /^[a-zA-Z0-9_ ]*$/
• Remove leading spaces using the .trim() method
• And if memo is empty just remove it from the transaction object

Do all of these in the createTransaction() method in the PaymentHandler class

This is the last thing needed for the migration to the account manager. I could make the updates on your branch if you want

I added those in

[tomijaga]

@AbhayAysola Nice!
@zinoadidi We can merge this in now and create new tasks for test and docs

[AbhayAysola]

I tested it and made some changes @zinoadidi @tomijaga

Should I update tests and docs in this PR or will it be in another task?

[zinoadidi]

@AbhayAysola you can go ahead and update test and docs as well; mention me when its ready and we can have this feature released as well;

Good job!

[AbhayAysola]

@zinoadidi I updated the docs but there wasn't anything to change in the tests since it is just an addition of a field.

[zinoadidi]

@AbhayAysola seen.
I do think the field should be reflected in the tests and transaction that don't meet the requirements should be included in test / updated to work with and reject memo as well
Else it will be possible to remove the field unknowingly in future and the tests will not catch it

[AbhayAysola]

@AbhayAysola seen.
I do think the field should be reflected in the tests and transaction that don't meet the requirements should be included in test / updated to work with and reject memo as well
Else it will be possible to remote the field unknowingly in future and the tests will not catch it

Hmm I guess you're right. I'll add that in

[zinoadidi]

@AbhayAysola confirm your account number

[AbhayAysola]

@AbhayAysola confirm your account number

8928526805de48e4bf2ed2a9b4b839e6b2603018ecdfbf4cffdb2065e01a3ed1