-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable mutiny plus profiles instead of delete #1173
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch,
Some(profile) => { | ||
if profile.tag != NwcProfileTag::Subscription { | ||
let mut nwc = profile.clone(); | ||
nwc.tag = NwcProfileTag::Subscription; | ||
self.nostr.edit_nwc_profile(nwc)?; | ||
} | ||
if let Some(nwc) = nwc { | ||
// only should have to submit the NWC if never created locally before | ||
subscription_client.submit_nwc(nwc).await?; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why delete this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's not needed. Why keep it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use this in the front end to mark it as a subscription so we know to not display the nwc connection string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should already be marked correctly and that's not how the frontend handles it. https://github.com/MutinyWallet/mutiny-web/blob/a4981cb8cf0d8afe6f7e5d540fac0cc6b9c858d4/src/routes/settings/Connections.tsx#L110-L118
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, its for the open in client buttons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should already be marked correctly
ah okay, this was for fixing old profiles, that's fine then
98f4e2e
to
7a9645f
Compare
This partially solves this problem: MutinyWallet/mutiny-web#1130
It at least lets you "disable" mutiny+ subscriptions. Nothing yet to make it enable though, but it at least doesn't charge people money that explicitly removed the old profile.
Basically we need to disable, not full remove, because we ensure that a profile exists for subscribers.