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

Review and revise saving to a repo you don't have writing permissions to. #51

Open
ilovan opened this issue Jun 20, 2019 · 8 comments
Open
Assignees

Comments

@ilovan
Copy link
Contributor

ilovan commented Jun 20, 2019

Expected Behaviour

Check for writing permissions before going through (this repo exists, etc.)

Steps to Reproduce (for bugs)

  1. Open a repo you don't have writing permission to (e.g. https://dev-cwrc-writer.cwrc.ca/?githubPath=document.xml&githubRepo=SusanBrown%2FDTOC-Template)
  2. Make a change
  3. Hit Save on the ribbon
    Result:

Jun-20-2019 09-53-15

@ajmacdonald ajmacdonald self-assigned this Jul 5, 2019
@ajmacdonald
Copy link
Collaborator

As part of this rework, I'm wondering if it makes sense to keep the additional "this repository exists - would you like to use it" confirmation? @ilovan

@ilovan
Copy link
Contributor Author

ilovan commented Jul 5, 2019

can you condense it into "this repository and document path exist - would you like to use them?

@ajmacdonald
Copy link
Collaborator

Not easily because those are two separate checks that happen at different points in the saving process. Not to mention the case where the repository exists but the document path doesn't.

I just think it seems a bit redundant to ask the user if they really want to use the repository that they've already specified.

@ilovan
Copy link
Contributor Author

ilovan commented Jul 5, 2019

right, but at the same time, the users should be warned if they are creating a new repo. So maybe instead of "this repository exists - would you like to use it" how about if the message "the repository you specified does not exist yet - would you like to create it?" (basically reverse the check)
maybe do the same for the second check as well.

@ajmacdonald
Copy link
Collaborator

I agree.

@ilovan
Copy link
Contributor Author

ilovan commented Jul 5, 2019

actually, the second file already follows the logic I just described for the repo check - so no need to change that.

@ajmacdonald
Copy link
Collaborator

@ilovan
Copy link
Contributor Author

ilovan commented Jan 22, 2020

confirmed on dev-cwrc-writer

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

No branches or pull requests

2 participants