Skip to content
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

Feat multiple accounts and instances #851

Merged
merged 74 commits into from
Jun 11, 2024

Conversation

byawitz
Copy link
Member

@byawitz byawitz commented May 21, 2024

What does this PR do?

  • Enabling multiple users and Appwrite instances, while keeping comparability with previous prefs.json scheme.
  • Appwrite cloud is now the default endpoint url, this can be changed by adding the --self-hosted flag.
  • Easy migrate login credentials with appwrite login migrate
  • Display all logins using appwrite login list
  • Change context using appwrite login change --account-id

Test Plan

multiple-accounts.mp4

@byawitz byawitz self-assigned this May 21, 2024
@byawitz byawitz changed the base branch from master to feat-cli-whoami-command May 21, 2024 18:49
@byawitz
Copy link
Member Author

byawitz commented May 22, 2024

Changing multiple account selection to the login command and making it interactive

multiple-accounts-interactive.mp4

@byawitz
Copy link
Member Author

byawitz commented May 22, 2024

Interactive multiple account logout

multiple-accounts-logout-interactive.mp4

Base automatically changed from feat-cli-whoami-command to feat-cli-g2 May 24, 2024 13:02
@Meldiron
Copy link
Contributor

appwrite login
If its first time: Normal
If already has account:
New or existring

appwrite logout
- If one, sign out
- If many, select (indicate current)

@byawitz byawitz requested a review from Meldiron May 24, 2024 15:07
@@ -99,4 +99,5 @@ const sdkForProject = async () => {
module.exports = {
sdkForConsole,
sdkForProject,
questionGetEndpoint,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets move all questions to questions.js file only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 5-28 has declared the array questionGetEndpoint

when: (answers) => answers.method === 'select'
},
];
const questionGetEndpoint = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems we already have questionGetEndpoint here. We have to remove it from sdks.js

}
}

const leftLogins = globalConfig.getLogins();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this method be called getSessions() ?

…nces' into feat-multiple-accounts-and-instances

# Conflicts:
#	templates/cli/lib/commands/push.js.twig
#	templates/cli/lib/config.js.twig

client.setCookie(globalConfig.getCookie());
globalConfig.addLogin(id, {});
globalConfig.setCurrentLogin(id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
globalConfig.setCurrentLogin(id);
globalConfig.setCurrentSession(id);

@@ -53,6 +79,8 @@ const loginCommand = async () => {
parseOutput: false
});
} else {
globalConfig.removeLogin(id);
globalConfig.setCurrentLogin(oldCurrent);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
globalConfig.setCurrentLogin(oldCurrent);
globalConfig.setCurrentSession(oldCurrent);

return;
}
if (logins.length === 1) {
await singleLogout(current);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also singleLogout is not a very good name. Lets just call it deleteSession or logout

Comment on lines 418 to 452
getCurrentLogin() {
if (!this.has(Global.PREFERENCE_CURRENT)) {
return "";
}
return this.get(Global.PREFERENCE_CURRENT);
}

setCurrentLogin(endpoint) {
this.set(Global.PREFERENCE_CURRENT, endpoint);
}

getLoginIds() {
return Object.keys(this.data).filter((key) => !Global.IGNORE_ATTRIBUTES.includes(key));
}

getLogins() {
const logins = Object.keys(this.data).filter((key) => !Global.IGNORE_ATTRIBUTES.includes(key))

return logins.map((login) => {

return {
id: login,
endpoint: this.data[login][Global.PREFERENCE_ENDPOINT],
email: this.data[login][Global.PREFERENCE_EMAIL]
}
})
}

addLogin(login, data) {
this.set(login, data);
}

removeLogin(login, data) {
this.delete(login);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets refactor all these method names to use Session / Session

And deleteSession / logout

this.set(Global.PREFERENCE_CURRENT, endpoint);
}

getLoginIds() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be getSessionIds() ?

}
}

const leftLogins = globalConfig.getSessions();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const leftLogins = globalConfig.getSessions();
const sessions = globalConfig.getSessions();

helpWidth: process.stdout.columns || 80
})
.action(actionRunner(async () => {
const logins = globalConfig.getSessions();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const logins = globalConfig.getSessions();
const sessions = globalConfig.getSessions();

@christyjacob4 christyjacob4 merged commit 11fe1ff into feat-console-flow Jun 11, 2024
35 checks passed
@christyjacob4 christyjacob4 deleted the feat-multiple-accounts-and-instances branch June 11, 2024 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants