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

Migration of zuliprc location to a config directory #1503

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
run: Create account-wise zuliprc files in the new config path.
Newly generated zuliprc files were being saved to `~/zuliprc`.
This moves default zuliprc creation into the user's config directory.

The --config-file option will no longer be able to save zuliprc files
in custom locations.

The new config path including the .config directory are handled safely,
to allow creation even if they do not already exist.

Tests updated.

The test with id: `path_does_not_exist`, in the test function
`test_main_cannot_write_zuliprc_given_good_credentials()` has been
removed, as the path will now be created even if it does not exist.
Niloth-p committed Aug 12, 2024
commit a4038b2b7260208cb83f2263afca94e86a0c0739
22 changes: 8 additions & 14 deletions tests/cli/test_run.py
Original file line number Diff line number Diff line change
@@ -378,32 +378,26 @@ def unreadable_dir(tmp_path: Path) -> Generator[Tuple[Path, Path], None, None]:
unreadable_dir.chmod(0o755)


@pytest.mark.parametrize(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed parametrize as there's only one test case remaining, moved the values inside the test function.
The test function could be renamed to be more specific too.

"path_to_use, expected_exception",
[
("unreadable", "PermissionError"),
("goodnewhome", "FileNotFoundError"),
],
ids=["valid_path_but_cannot_be_written_to", "path_does_not_exist"],
)
def test_main_cannot_write_zuliprc_given_good_credentials(
capsys: CaptureFixture[str],
mocker: MockerFixture,
unreadable_dir: Tuple[Path, Path],
path_to_use: str,
expected_exception: str,
path_to_use: str = "unreadable",
expected_exception: str = "PermissionError",
) -> None:
tmp_path, unusable_path = unreadable_dir

# This is default base path to use
zuliprc_path = os.path.join(str(tmp_path), path_to_use, "zuliprc")
mocker.patch(MODULE + ".HOME_PATH_ZULIPRC", zuliprc_path)
mocker.patch(MODULE + ".CONFIG_PATH", os.path.join(str(tmp_path), "config"))
mocker.patch(MODULE + ".CONFIG_PATH", zuliprc_path)
mocker.patch(MODULE + ".HOME_PATH_ZULIPRC", os.path.join(str(tmp_path), "home"))
account_alias = "my_account_alias"

# Give some arbitrary input and fake that it's always valid
mocker.patch.object(builtins, "input", lambda _: "text\n")
mocker.patch(
MODULE + ".get_api_key", return_value=("my_site", "my_login", "my_api_key")
MODULE + ".get_api_key",
return_value=("my_site", "my_login", "my_api_key", account_alias),
)

with pytest.raises(SystemExit):
@@ -415,7 +409,7 @@ def test_main_cannot_write_zuliprc_given_good_credentials(
expected_line = (
"\x1b[91m"
f"{expected_exception}: zuliprc could not be created "
f"at {zuliprc_path}"
f"at {os.path.join(zuliprc_path, account_alias, 'zuliprc')}"
"\x1b[0m"
)
assert lines[-1] == expected_line
28 changes: 22 additions & 6 deletions zulipterminal/cli/run.py
Original file line number Diff line number Diff line change
@@ -145,7 +145,7 @@ def parse_args(argv: List[str]) -> argparse.Namespace:
"-c",
action="store",
help="config file downloaded from your zulip "
f"organization (default: {HOME_PATH_ZULIPRC})",
f"organization (default: {CONFIG_PATH}/account_alias/zuliprc)",
)
parser.add_argument(
"--theme",
@@ -266,7 +266,7 @@ def get_server_settings(realm_url: str) -> ServerSettings:
return response.json()


def get_api_key(realm_url: str) -> Optional[Tuple[str, str, str]]:
def get_api_key(realm_url: str) -> Optional[Tuple[str, str, str, str]]:
from getpass import getpass

try:
@@ -281,6 +281,12 @@ def get_api_key(realm_url: str) -> Optional[Tuple[str, str, str]]:
login_id_label = get_login_label(server_properties)
login_id = styled_input(login_id_label)
password = getpass(in_color("blue", "Password: "))
account_alias = styled_input(
"Please choose a simple and unique name for this account,"
" which represents your user profile and its server."
" Use only letters, numbers, and underscores.\n"
"Account alias: "
)

response = requests.post(
url=f"{preferred_realm_url}/api/v1/fetch_api_key",
@@ -290,7 +296,12 @@ def get_api_key(realm_url: str) -> Optional[Tuple[str, str, str]]:
},
)
if response.status_code == requests.codes.OK:
return preferred_realm_url, login_id, str(response.json()["api_key"])
return (
preferred_realm_url,
login_id,
str(response.json()["api_key"]),
account_alias,
)
return None


@@ -321,13 +332,13 @@ def login_and_save(zuliprc_path: Optional[str]) -> str:
# for adding "/api"
realm_url = realm_url.rstrip("/")
login_data = get_api_key(realm_url)
zuliprc_path = zuliprc_path or path.expanduser(HOME_PATH_ZULIPRC)

while login_data is None:
print(in_color("red", "\nIncorrect Email(or Username) or Password!\n"))
login_data = get_api_key(realm_url)

preferred_realm_url, login_id, api_key = login_data
preferred_realm_url, login_id, api_key, account_alias = login_data
zuliprc_path = os.path.join(CONFIG_PATH, account_alias, "zuliprc")
save_zuliprc_failure = _write_zuliprc(
zuliprc_path,
login_id=login_id,
@@ -349,6 +360,7 @@ def _write_zuliprc(
Only creates new private files; errors if file already exists
"""
try:
Path(to_path).parent.mkdir(parents=True, exist_ok=True, mode=0o700)
with open(
os.open(to_path, os.O_CREAT | os.O_WRONLY | os.O_EXCL, 0o600), "w"
) as f:
@@ -388,7 +400,11 @@ def resolve_to_valid_path(zuliprc_str: Optional[str]) -> str:


def check_for_default_zuliprc() -> Optional[str]:
zuliprc_count_in_config = sum(1 for _ in Path(CONFIG_PATH).glob("*/zuliprc"))
zuliprc_count_in_config = (
sum(1 for _ in Path(CONFIG_PATH).glob("*/zuliprc"))
if path.exists(CONFIG_PATH)
else 0
)
home_path_count = path.exists(HOME_PATH_ZULIPRC)
total_count = zuliprc_count_in_config + home_path_count