Skip to content
Snippets Groups Projects

Favor transaction functions

Closed Hannes Heine requested to merge pr2433head into pr2433base

Created by: Tirokk

mattwr18 Authored by mattwr18 Merged


:cake: Pullrequest

Issues

Todo

Middleware

  • permissionsMiddleware.js
  • sluggifyMiddleware.js
  • validationMiddleware.js

Resolvers

  • locations.js
  • comments.js
  • createPasswordReset.js
  • existingEmailAddress.js
  • notifications.js
  • passwordReset.js
  • posts.js
  • rewards.js
  • shout.js
  • statistics.js
  • user_management.js
  • users.js

Tests

  • Add tests for middleware/nodes/locations.js
  • passwordReset.spec.js

Factories

  • index.js

Merge request reports

Closed by avatar (Jul 26, 2025 2:42pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
21 5 const session = context.driver.session()
6
22 7 try {
23 await session.run(cypherDeletePreviousRelations, {
24 postId,
25 })
26 await session.run(cypherCreateNewTagsAndRelations, {
27 postId,
28 hashtags,
8 await session.writeTransaction(txc => {
9 return txc.run(
10 `
11 MATCH (post:Post { id: $postId})
12 OPTIONAL MATCH (post)-[previousRelations:TAGGED]->(tag:Tag)
13 DELETE previousRelations
14 WITH post
  • Created by: Tirokk

    roschaefer Authored by roschaefer


    Outdated (history rewrite) - original diff


    @@ -2,30 +2,23 @@ import extractHashtags from '../hashtags/extractHashtags'
     
     const updateHashtagsOfPost = async (postId, hashtags, context) => {
       if (!hashtags.length) return
    -
    -  // We need two Cypher statements, because the 'MATCH' in the 'cypherDeletePreviousRelations' statement
    -  //  functions as an 'if'. In case there is no previous relation, the rest of the commands are omitted
    -  //  and no new Hashtags and relations will be created.
    -  const cypherDeletePreviousRelations = `
    -    MATCH (p: Post { id: $postId })-[previousRelations: TAGGED]->(t: Tag)
    -    DELETE previousRelations
    -    RETURN p, t
    -    `
    -  const cypherCreateNewTagsAndRelations = `
    -    MATCH (p: Post { id: $postId})
    -    UNWIND $hashtags AS tagName
    -    MERGE (t: Tag { id: tagName, disabled: false, deleted: false })
    -    MERGE (p)-[:TAGGED]->(t)
    -    RETURN p, t
    -    `
       const session = context.driver.session()
    +
       try {
    -    await session.run(cypherDeletePreviousRelations, {
    -      postId,
    -    })
    -    await session.run(cypherCreateNewTagsAndRelations, {
    -      postId,
    -      hashtags,
    +    await session.writeTransaction(txc => {
    +      return txc.run(
    +        ` 
    +          MATCH (post:Post { id: $postId})
    +          OPTIONAL MATCH (post)-[previousRelations:TAGGED]->(tag:Tag)
    +          DELETE previousRelations
    +          WITH post

    Very good!

  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit 30707677
  • 119 export const validateNotifyUsers = async (label, reason) => {
    120 const reasonsAllowed = ['mentioned_in_post', 'mentioned_in_comment', 'commented_on_post']
    121 if (!reasonsAllowed.includes(reason)) throw new Error('Notification reason is not allowed!')
    122 if (
    123 (label === 'Post' && reason !== 'mentioned_in_post') ||
    124 (label === 'Comment' && !['mentioned_in_comment', 'commented_on_post'].includes(reason))
    125 ) {
    126 throw new Error('Notification does not fit the reason!')
    127 }
    128 }
    129
    130 const validateUpdateUser = async (resolve, root, params, context, info) => {
    131 const { name } = params
    132 if (typeof name === 'string' && name.trim().length < USERNAME_MIN_LENGTH)
    133 throw new UserInputError(`Username must be at least ${USERNAME_MIN_LENGTH} character long!`)
    134 return resolve(root, params, context, info)
    • Created by: Tirokk

      roschaefer Authored by roschaefer


      Outdated (history rewrite) - original diff


      @@ -115,12 +116,32 @@ const validateReview = async (resolve, root, args, context, info) => {
         return resolve(root, args, context, info)
       }
       
      +export const validateNotifyUsers = async (label, reason) => {
      +  const reasonsAllowed = ['mentioned_in_post', 'mentioned_in_comment', 'commented_on_post']
      +  if (!reasonsAllowed.includes(reason)) throw new Error('Notification reason is not allowed!')
      +  if (
      +    (label === 'Post' && reason !== 'mentioned_in_post') ||
      +    (label === 'Comment' && !['mentioned_in_comment', 'commented_on_post'].includes(reason))
      +  ) {
      +    throw new Error('Notification does not fit the reason!')
      +  }
      +}
      +
      +const validateUpdateUser = async (resolve, root, params, context, info) => {
      +  const { name } = params
      +  if (typeof name === 'string' && name.trim().length > USERNAME_MIN_LENGTH)
      +    return resolve(root, params, context, info)
      +  if (typeof name !== 'string' || name.trim().length < USERNAME_MIN_LENGTH)

      I would say just remove the if check, no? Is there a reason for it?

  • Hannes Heine
    Hannes Heine @Elweyn started a thread on commit 30707677
  • 172 175 createdAt: expect.any(String),
    173 176 },
    174 177 },
    178 errors: undefined,
    • Created by: Tirokk

      roschaefer Authored by roschaefer


      Outdated (history rewrite) - original diff


      @@ -172,6 +175,7 @@ describe('UpdateComment', () => {
                     createdAt: expect.any(String),
                   },
                 },
      +          errors: undefined,

      Good! We should do that more often.

  • Created by: Mogge

    Review: Approved

    Looks good to me! Feel free to add the few suggestions.

    looks good to me

  • Please register or sign in to reply
    Loading