-
Notifications
You must be signed in to change notification settings - Fork 12
New client app ticket index #157
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
base: master
Are you sure you want to change the base?
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.
Hi, I only had a very quick look but it seems to be a completely different project than the current Zendesk plugin. Can you give more background about the project?
If I understand correctly, it's a standalone Node.js server that will receive webhooks to index Zendesk tickets. It might be easier to bootstrap a new repo for that actually.
Regarding the hosting, each teams does it differently, you'd have to check with yours, or contact Foundation.
Otherwise for the PR itself, it would need at least some detailed "How to test" instructions.
@@ -0,0 +1,27 @@ | |||
-----BEGIN RSA PRIVATE KEY----- |
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.
You can't commit a private key, especially on a public repo, it needs to be stored securely.
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.
I'll work out a way to test it, although not having a server, that makes it difficult. I'll see about contacting Foundation, or seeing about Heroku or something similar. Thanks for the info. Oh, and I forgot about that server.key...I'll fix. Thanks @sbellone
const functions = require('./indexFunctions'); | ||
|
||
app.get('/', (req, res) => { | ||
res.send("Hello world"); |
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.
👋
|
||
app.use(express.static('public')); | ||
|
||
app.get('/webhook', (req, res) => { |
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.
Who will call that webhook?
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.
Zendesk itself will call it when a ticket is updated. It passes in the ticketId
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.
Ok, and how does it know the server address? This kind of setup needs to be documented
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.
true..I can update the docs with screenshots and the like
const historyRouter = instantsearch.routers.history(); | ||
|
||
|
||
const searchClient = algoliasearch(settings.algoliaAppName, settings.searchApiKey); |
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.
algoliaAppName
is not a good name if it's actually an AppID
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.
thanks..I agree.
This is a pull request that contains several things.
These 2 updates are improvements, because I think we should be indexing and searching for tickets as well as articles.
Questions:
Feel free to reach out with any questions of your own.