meteorlxy / vssue

:mailbox: A Vue-powered Issue-based Comment Plugin
https://vssue.js.org
MIT License
770 stars 106 forks source link

Vssue "browser" version automatically tries to log in to GitHub #32

Closed mattbishop closed 5 years ago

mattbishop commented 5 years ago

Describe the bug

The "in-browser" Getting Started section here: https://vssue.js.org/guide/getting-started.html#in-browser

I've chosen:

Expected behavior

I would expect the client would have to log into github only when they want to, not automatically.

Environment (please complete the following information):

Additional context

I have left this running at http://level3.rest

meteorlxy commented 5 years ago

Check if this option helps : https://vssue.js.org/options/#autocreateissue

And this tip:

image

mattbishop commented 5 years ago

Thank you, setting "autoCreateIssue" to false stopped the auto login issue.

Now, however, it cannot create an issue with a comment if the issue does not exist already. In "postComment", there is a test for issue having a value, and when I debug I see that issue = null:

if (!this.API || !this.issue || this.isLoadingComments)

Not undefined, but null.

meteorlxy commented 5 years ago

Not undefined, but null.

What's wrong with this?

mattbishop commented 5 years ago

Just clarifying that it is null instead of undefined.

The full method in Chrome is:

        async postComment({content: e}) {
            try {
                if (!this.API || !this.issue || this.isCreatingComment)
                    return;
                return this.isCreatingComment = !0,
                await this.API.postComment({accessToken:this.accessToken,content:e,issueId:this.issue.id})
            } catch (e) {
                throw this.$emit("error", e),
                e
            } finally {
                this.isCreatingComment = !1
            }
        }

Sorry I cannot find it in your source code. I'm guessing webpack magic is making it hard to find. :)

mattbishop commented 5 years ago

So I need to figure out why issue is null. My options look like this (I left out github props):

        options: {
          perPage: 20,
          autoCreateIssue: false,
          prefix: '[Comment]',
          labels: ['comment'],
...

So I am using default title and a specific label. I have created the label in my repo's issues.

meteorlxy commented 5 years ago

Yes it's intended to be null:

https://github.com/meteorlxy/vssue/blob/8488cf4512a5610ec79eda5ee1df09b9c4341785/packages/vssue/src/VssueStore.ts#L134

Source code here:

https://github.com/meteorlxy/vssue/blob/8488cf4512a5610ec79eda5ee1df09b9c4341785/packages/vssue/src/VssueStore.ts#L276-L299

meteorlxy commented 5 years ago

You can check these methods if you want to figure out why the issue is not found

https://github.com/meteorlxy/vssue/blob/8488cf4512a5610ec79eda5ee1df09b9c4341785/packages/vssue/src/VssueStore.ts#L171-L235

meteorlxy commented 5 years ago

And this tip may help

image

mattbishop commented 5 years ago

I think the problem is when autoCreateIssue=false AND no matching issue exists in GitHub, Vssue will not POST a new issue when the user clicks "submit comment". I am expecting "submit comment" to POST to create the issue.

mattbishop commented 5 years ago

I have 0 issues: https://github.com/Level3-REST/site/issues

meteorlxy commented 5 years ago

I am expecting "submit comment" to POST to create the issue.

It's not so reasonable. We should not allow all users to create an issue for your page. In addition, the other users cannot access labels in your repository.

That's why we use labels to help identify the issue, or other people can create a fake issue to hack your page.

mattbishop commented 5 years ago

I see your reasoning, yet I'd be OK to accept that any logged-in github user could create a new issue for my page. They can already to that in the "issues" tab for my project so I don't see a difference.

Ideally labels could be templatized like title to help with finding them. like, 'comment: ${url}'

meteorlxy commented 5 years ago

They can create a new issue, but they cannot set the labels.

meteorlxy commented 5 years ago

We can keep discussion if you have any ideas. Close this issue as the original problem is fixed.

mattbishop commented 5 years ago

Thanks for being so responsive! Vssues is a great project.