feat: added featured app revoke - #2114
Conversation
Signed-off-by: Phillip Olesen [Cashen] <phillip@cashen.cc>
|
hey @rukmini-basu-da thanks for approving and merging with main! I am not able to start the workflows (they are awaiting approval) is that something you can do ? |
Signed-off-by: Phillip Olesen [Cashen] <phillip@cashen.cc>
|
build had gotten broken because of this pr: I have updated a small fix above, @rukmini-basu-da can you approve workflows again ? |
mateuszpiatkowski-da
left a comment
mateuszpiatkowski-da
left a comment
There was a problem hiding this comment.
Hey @phillip-cashen! Thank you for your contribution to our project!
I've managed to review you PR after the changes and left some comments. Also, I've noticed that there are some pipeline scripts that are in red and we'll have to address that as well. :)
| '6c5802f86709a0ad4784af81f0bab40f3070b2f58128d8843da1e1784c147802:Splice.Amulet:FeaturedAppRight' | ||
| ) | ||
|
|
||
| expect(command.choice).toEqual('FeaturedAppRight_Cancel') |
There was a problem hiding this comment.
| expect(command.choice).toEqual('FeaturedAppRight_Cancel') | |
| expect(command).toStrictEquak({ | |
| // full object properties | |
| }) |
This will ensure that the function returns the object of desired interface :)
There was a problem hiding this comment.
i have updated it to use strictEqual 👍
| } | ||
| } | ||
|
|
||
| async function fetchViaNodeHttp( |
There was a problem hiding this comment.
Truth be told I'm not sure I understand the logic flow here.
Could I ask you to simplify the logic and, if not possible, document it properly so that it can shine some light for other visitors? JSDoc would be fantastic :)
There was a problem hiding this comment.
I'll add the JSDOC to explain the flow 👍
the TLDR:
we basically just take the the string, split it into its comments (scheme, port, path & query) and then reconstruct it with a bypass for IPv6 (::1) and setting the localnet nginx to match the vhost.
| options?: GrantFeaturedAppRightsOptions | ||
| ) => Promise<FeaturedAppRight | undefined> | ||
| /** | ||
| * Submits a command to revoke featured app rights for validator operator. |
There was a problem hiding this comment.
I think we can remove this jsdoc since it's already attached to implementation in namespace.ts
Signed-off-by: Phillip Olesen [Cashen] <phillip@cashen.cc>
3 participants
hey Wallet Team,
hope you are doing well, super cool repo you have here!
i took the liberty of adding a new feature to the featured app control, namely for revoking a featured app status. I tried as hard as possible to ensure it follow existing patterns and behavior of the repo.
i also added a little helper to scan-api that handles the annoyance of
scan.localhostwithout needing to changes/etc/hosts/files for the mapping (this makes it a lot easier to run the code for people not familiar with the repo).Looking forward to seeing your feedback and thoughts !