lexicongovernance / pluraltools-backend

Backend implementation of the plural voting tool.
https://demo.lexicongovernance.org/
GNU General Public License v3.0
3 stars 1 forks source link

Refactor vote service #439

Closed MartinBenediktBusch closed 2 months ago

MartinBenediktBusch commented 3 months ago

A lot of functionality has been added to the vote service. Therefore, it would be good to refactor the vote service to increase readability and efficiency. Furthermore, the test suite for the vote service should be updated along with the refactor. How to best refactor the vote service must be discussed.

To Do

  1. Seperate functionalites further to make them more modular and therefore easier to test
  2. Implement consistent error handling
MartinBenediktBusch commented 3 months ago

The validateAndSaveVote functionality of the saveVotes function should be moved out:

for (const vote of data) {
    const { data, error } = await validateAndSaveVote(dbPool, vote, userId);
    if (data) {
      voteData.push(data);
    }
    if (error) {
      errors.push(error);
    }
  }

Instead the saveVotes function should take the output of the ValidateAndSaveVote function as an input. In turn the ValidateAndSaveVote function should take the data as an input. I would move the validation itself into the handler to allow independent testing of the saveVotes and ValidateAndSaveVote function.

MartinBenediktBusch commented 3 months ago

Require a group categoryId on the questionsToGroupCategories table:

export const questionsToGroupCategories = pgTable('questions_to_group_categories', {
  id: uuid('id').primaryKey().defaultRandom(),
  questionId: uuid('question_id')
    .notNull()
    .references(() => forumQuestions.id),
  groupCategoryId: uuid('group_category_id').references(() => groupCategories.id), // Must be nullable (for now) because affiliation does not have a group category id.
  createdAt: timestamp('created_at').notNull().defaultNow(),
  updatedAt: timestamp('updated_at').notNull().defaultNow(),
});
MartinBenediktBusch commented 3 months ago

Improve the error handling in the queryGroupCategories function from:

if (groupCategoryIds.length === 0) {
    console.error('Group Category ID is Missing');
    return [];
  }

to something like this: return { data: voteData, errors }

MartinBenediktBusch commented 3 months ago

To calculate the plurality score the questionId should be queried once instead of each time the plurality score gets calculated:

/**
 * Updates the vote score for a specific option in the database according to the plural voting model.
 *
 * This function queries vote and multiplier data from the database,
 * combines them, calculates the score using plural voting, updates
 * the vote score in the database, and returns the calculated score.
 *
 * @param { NodePgDatabase<typeof db>} dbPool - The database connection pool.
 * @param {string} optionId - The ID of the option for which to update the vote score.
 */
export async function updateVoteScorePlural(
  dbPool: NodePgDatabase<typeof db>,
  optionId: string,
): Promise<number> {
  // Query and transform vote data
  const voteArray = await queryVoteData(dbPool, optionId);
  const votesDictionary = await numOfVotesDictionary(voteArray);

  // Query group data, grouping dimensions, and calculate the score
  const queryQuestionId = await dbPool
    .select({
      questionId: db.questionOptions.questionId,
    })
    .from(db.questionOptions)
    .where(eq(db.questionOptions.id, optionId));

  const groupCategories = await queryGroupCategories(dbPool, queryQuestionId[0]!.questionId);
  const groupArray = await groupsDictionary(dbPool, votesDictionary, groupCategories ?? []);
  const score = await calculatePluralScore(groupArray, votesDictionary);

  await updateVoteScoreInDatabase(dbPool, optionId, score);

  return score;
}
MartinBenediktBusch commented 3 months ago

Move cycle validation out of the saveVote function as the vote function should only be responsible for saving votes and not for validation. Validation should be handled by ValidateAndSaveVote directly:

/**
 * Saves a vote in the database.
 *
 * This function checks if the cycle for the given question is open,
 * then inserts the provided vote data into the database and returns the saved vote data.
 *
 * @param { NodePgDatabase<typeof db>} dbPool - The database connection pool.
 * @param {z.infer<typeof insertVotesSchema>} vote - The vote data to be saved.
 */
export async function saveVote(
  dbPool: NodePgDatabase<typeof db>,
  vote: z.infer<typeof insertVotesSchema>,
) {
  // check if cycle is open
  const queryQuestion = await dbPool.query.forumQuestions.findFirst({
    where: eq(db.forumQuestions.id, vote?.questionId ?? ''),
    with: {
      cycle: true,
    },
  });

  if ((queryQuestion?.cycle?.status as CycleStatusType) !== 'OPEN') {
    return { errors: [{ message: 'Cycle is not open' }] };
  }

  // save the votes
  const newVote = await dbPool
    .insert(votes)
    .values({
      userId: vote.userId,
      numOfVotes: vote.numOfVotes,
      optionId: vote.optionId,
      questionId: vote.questionId,
    })
    .returning();

  return { data: newVote[0] };
}
MartinBenediktBusch commented 3 months ago

I think the definition of the insertVoteBody schema should be part of the saveVote function and not the SaveAndValidate vote function as it is related to voting:

 const insertVoteBody: z.infer<typeof insertVotesSchema> = {
    optionId: vote.optionId,
    numOfVotes: vote.numOfVotes,
    userId: userId,
    questionId: queryQuestionOption.questionId,
  };

  const body = insertVotesSchema.safeParse(insertVoteBody);

  if (!body.success) {
    return { data: null, error: body.error.errors[0]?.message };
  }
MartinBenediktBusch commented 3 months ago

Require a group categoryId on the questionsToGroupCategories table:

export const questionsToGroupCategories = pgTable('questions_to_group_categories', {
  id: uuid('id').primaryKey().defaultRandom(),
  questionId: uuid('question_id')
    .notNull()
    .references(() => forumQuestions.id),
  groupCategoryId: uuid('group_category_id').references(() => groupCategories.id), // Must be nullable (for now) because affiliation does not have a group category id.
  createdAt: timestamp('created_at').notNull().defaultNow(),
  updatedAt: timestamp('updated_at').notNull().defaultNow(),
});

Done

MartinBenediktBusch commented 3 months ago

Improve the error handling in the queryGroupCategories function from:

if (groupCategoryIds.length === 0) {
    console.error('Group Category ID is Missing');
    return [];
  }

to something like this: return { data: voteData, errors }

Done

MartinBenediktBusch commented 3 months ago

To calculate the plurality score the questionId should be queried once instead of each time the plurality score gets calculated:

/**
 * Updates the vote score for a specific option in the database according to the plural voting model.
 *
 * This function queries vote and multiplier data from the database,
 * combines them, calculates the score using plural voting, updates
 * the vote score in the database, and returns the calculated score.
 *
 * @param { NodePgDatabase<typeof db>} dbPool - The database connection pool.
 * @param {string} optionId - The ID of the option for which to update the vote score.
 */
export async function updateVoteScorePlural(
  dbPool: NodePgDatabase<typeof db>,
  optionId: string,
): Promise<number> {
  // Query and transform vote data
  const voteArray = await queryVoteData(dbPool, optionId);
  const votesDictionary = await numOfVotesDictionary(voteArray);

  // Query group data, grouping dimensions, and calculate the score
  const queryQuestionId = await dbPool
    .select({
      questionId: db.questionOptions.questionId,
    })
    .from(db.questionOptions)
    .where(eq(db.questionOptions.id, optionId));

  const groupCategories = await queryGroupCategories(dbPool, queryQuestionId[0]!.questionId);
  const groupArray = await groupsDictionary(dbPool, votesDictionary, groupCategories ?? []);
  const score = await calculatePluralScore(groupArray, votesDictionary);

  await updateVoteScoreInDatabase(dbPool, optionId, score);

  return score;
}

Done

MartinBenediktBusch commented 3 months ago

Move cycle validation out of the saveVote function as the vote function should only be responsible for saving votes and not for validation. Validation should be handled by ValidateAndSaveVote directly:

/**
 * Saves a vote in the database.
 *
 * This function checks if the cycle for the given question is open,
 * then inserts the provided vote data into the database and returns the saved vote data.
 *
 * @param { NodePgDatabase<typeof db>} dbPool - The database connection pool.
 * @param {z.infer<typeof insertVotesSchema>} vote - The vote data to be saved.
 */
export async function saveVote(
  dbPool: NodePgDatabase<typeof db>,
  vote: z.infer<typeof insertVotesSchema>,
) {
  // check if cycle is open
  const queryQuestion = await dbPool.query.forumQuestions.findFirst({
    where: eq(db.forumQuestions.id, vote?.questionId ?? ''),
    with: {
      cycle: true,
    },
  });

  if ((queryQuestion?.cycle?.status as CycleStatusType) !== 'OPEN') {
    return { errors: [{ message: 'Cycle is not open' }] };
  }

  // save the votes
  const newVote = await dbPool
    .insert(votes)
    .values({
      userId: vote.userId,
      numOfVotes: vote.numOfVotes,
      optionId: vote.optionId,
      questionId: vote.questionId,
    })
    .returning();

  return { data: newVote[0] };
}

Done

MartinBenediktBusch commented 3 months ago

I think the definition of the insertVoteBody schema should be part of the saveVote function and not the SaveAndValidate vote function as it is related to voting:

 const insertVoteBody: z.infer<typeof insertVotesSchema> = {
    optionId: vote.optionId,
    numOfVotes: vote.numOfVotes,
    userId: userId,
    questionId: queryQuestionOption.questionId,
  };

  const body = insertVotesSchema.safeParse(insertVoteBody);

  if (!body.success) {
    return { data: null, error: body.error.errors[0]?.message };
  }

Done

MartinBenediktBusch commented 3 months ago

The validateAndSaveVote functionality of the saveVotes function should be moved out:

for (const vote of data) {
    const { data, error } = await validateAndSaveVote(dbPool, vote, userId);
    if (data) {
      voteData.push(data);
    }
    if (error) {
      errors.push(error);
    }
  }

Instead the saveVotes function should take the output of the ValidateAndSaveVote function as an input. In turn the ValidateAndSaveVote function should take the data as an input. I would move the validation itself into the handler to allow independent testing of the saveVotes and ValidateAndSaveVote function.

Done