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.

Sign up for GitHub

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(gatsby-source-drupal): secrets and delete functionality #18345

Merged
merged 11 commits into from
Oct 23, 2019
Merged

feat(gatsby-source-drupal): secrets and delete functionality #18345

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
108d1e5
added functionality to delete nodes and enforce secret for more secur…
Oct 8, 2019
a78598a
added documentation for secret in config
Oct 8, 2019
7ef9603
updated handleWebhookData and destructured varaibled from request
Oct 8, 2019
941d6f4
using deleteNode from actions
Oct 8, 2019
d0f9c29
deleteNode working and reporting deleted node to user
Oct 8, 2019
f5442f8
updated documentation on using secrets to be consistent with the rest…
grantglidewell Oct 9, 2019
d6d5500
change destructuring due to failing tests
grantglidewell Oct 11, 2019
9087af2
Merge branch 'master' of github.com:gatsbyjs/gatsby into topic/source…
grantglidewell Oct 11, 2019
de6eaa9
Merge branch 'master' of github.com:gatsbyjs/gatsby into topic/source…
grantglidewell Oct 22, 2019
41e3748
fix undefined res
grantglidewell Oct 22, 2019
ffd774e
added return for lint
grantglidewell Oct 22, 2019
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
1 change: 1 addition & 0 deletions packages/gatsby-source-drupal/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ module.exports = {
baseUrl: `https://live-contentacms.pantheonsite.io/`,
apiBase: `api`, // optional, defaults to `jsonapi`
concurrentFileRequests: 60, // optional, defaults to `20`
secret: `xxxx-xxxx-xxxxx`, // optional, must match Drupal instance preview secret
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this exist in an environment variable rather than committed in code for security purposes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totally agree, Ill update the documentation to reflect that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I think the documentation is much clearer and consistent with the rest of the readme file now.

},
},
],
Expand Down
20 changes: 14 additions & 6 deletions packages/gatsby-source-drupal/src/gatsby-node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,21 @@ exports.onCreateDevServer = (
bodyParser.text({
type: `application/json`,
}),
async (req, res) => {
// we are missing handling of node deletion
const nodeToUpdate = JSON.parse(JSON.parse(req.body)).data

await handleWebhookUpdate(
async (req, _res) => {
const requestBody = JSON.parse(JSON.parse(req.body))
const { data, secret, action, id } = requestBody
grantglidewell marked this conversation as resolved.
Show resolved Hide resolved
if (pluginOptions.secret && pluginOptions.secret !== secret) {
return reporter.warn(
`The secret in this request did not match your plugin options secret.`
)
}
if (action === `delete`) {
actions.deleteNode({ node: getNode(createNodeId(id)) })
Copy link
Contributor

Choose a reason for hiding this comment

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

I do have question here:
What happens if given node is being referenced (relationship) by other node? Does drupal automatically update other posts that reference deleted content and remove those? Or are those not touched and then there is content that still references deleted content?

We definitely will need to add some code here to handle back references (I can work on that), question is if we need to handle normal references as well or drupal actually updates those automatically

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a great question. Ive asked for some insight into this on drupal, but I think a modified/extended version of what we already use to update relationships should be able to handle this.

Is this something deleteNode itself should/could handle?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is something that gatsby core potentially could handle, but doesn't do it right now (it's up to plugins to keep track of relationships - mostly because it's usually more efficient than any generic solution we could have in core)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems from my conversation with someone who knows Drupal, the relationship are not necessarily updated on delete and it depends on how content and relationships are set up as to when and how that update or removal may happen. ugh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here is an issue discussing the problem https://www.drupal.org/project/drupal/issues/2723323
Some of the fixes purely deal with Drupal view which would solve for JSON API problems.

return reporter.log(`Deleted node: ${id}`)
}
return await handleWebhookUpdate(
{
nodeToUpdate,
data,
actions,
cache,
createNodeId,
Expand Down
4 changes: 2 additions & 2 deletions packages/gatsby-source-drupal/src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ exports.handleReferences = handleReferences

const handleWebhookUpdate = async (
{
nodeToUpdate,
data,
actions,
cache,
createNodeId,
Expand All @@ -96,7 +96,7 @@ const handleWebhookUpdate = async (
) => {
const { createNode } = actions

const newNode = nodeFromData(nodeToUpdate, createNodeId)
const newNode = nodeFromData(data, createNodeId)

const nodesToUpdate = [newNode]

Expand Down