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.

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

Fix config #45

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@
"prettier": "^2.7.1",
"probot": "^12.2.4",
"tsx": "^3.12.7",
"yaml": "^2.2.2",
"zod": "^3.22.4"
"yaml": "^2.2.2"
},
"devDependencies": {
"@types/dotenv": "^8.2.0",
Expand Down
10 changes: 5 additions & 5 deletions src/handlers/assign/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ export async function startCommandHandler(context: Context) {
}

// Filter out labels that match the time labels defined in the config
const timeLabelsAssigned: Label[] = labels.filter((label) =>
typeof label === "string" || typeof label === "object"
? config.labels.time.some((item) => item.name === label.name)
const timeLabelsAssigned: Label[] = labels.filter((assignedLabel) =>
typeof assignedLabel === "string" || typeof assignedLabel === "object"
? config.labels.time.some((label) => label === assignedLabel.name)
: false
);

Expand All @@ -44,8 +44,8 @@ export async function startCommandHandler(context: Context) {
// Sort labels by weight and select the one with the smallest weight
const sortedLabels = timeLabelsAssigned
.sort((a, b) => {
const fullLabelA = labels.find((label) => label.name === a.name);
const fullLabelB = labels.find((label) => label.name === b.name);
const fullLabelA = labels.find((label) => label.name === a.name)?.name;
const fullLabelB = labels.find((label) => label.name === b.name)?.name;

if (!fullLabelA || !fullLabelB) {
return 0; // return a default value
Expand Down
4 changes: 2 additions & 2 deletions src/handlers/comment/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ export async function commentCreatedOrEdited(context: Context) {
const { id, handler } = userCommand;
logger.info("Running a comment handler", { id, handler: handler.name });

const feature = config.commands.find((e) => e.name === id.split("/")[1]);
const disabled = config.disabledCommands.some((command) => command === id.replace("/", ""));

if (feature?.enabled === false && id !== "/help") {
if (disabled && id !== "/help") {
return logger.warn("Skipping because it is disabled on this repo.", { id });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function getTimeLabelsAssigned(payload: Payload, config: BotConfig) {
const _labelType = typeof _label;
const _labelName = _labelType === "string" ? _label.toString() : _labelType === "object" ? _label.name : "unknown";

const timeLabel = timeLabelsDefined.find((item) => item.name === _labelName);
const timeLabel = timeLabelsDefined.find((label) => label === _labelName);
if (timeLabel) {
timeLabelsAssigned.push(_label);
}
Expand Down
10 changes: 5 additions & 5 deletions src/handlers/comment/handlers/assign/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ export async function assign(context: Context, body: string) {
const {
miscellaneous: { maxConcurrentTasks },
timers: { taskStaleTimeoutDuration },
commands,
disabledCommands,
} = context.config;

const startEnabled = commands.find((command) => command.name === "start");
const startDisabled = disabledCommands.some((command) => command === "start");

logger.info("Received '/start' command", { sender: payload.sender.login, body });

if (!issue) {
throw logger.warn(`Skipping '/start' because of no issue instance`);
}

if (!startEnabled?.enabled) {
if (startDisabled) {
throw logger.warn("The `/assign` command is disabled for this repository.");
}

Expand Down Expand Up @@ -74,13 +74,13 @@ export async function assign(context: Context, body: string) {
const labels = issue.labels;
const priceLabel = labels.find((label) => label.name.startsWith("Price: "));

let duration = null;
let duration: number | null = null;
if (!priceLabel) {
throw logger.warn("Skipping '/start' since no price label is set to calculate the timeline", priceLabel);
} else {
const timeLabelsAssigned = getTimeLabelsAssigned(payload, config);
if (timeLabelsAssigned) {
duration = calculateDurations(timeLabelsAssigned).shift();
duration = calculateDurations(timeLabelsAssigned).shift() || null;
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/handlers/comment/handlers/help.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export async function listAvailableCommands(context: Context, body: string) {

export function generateHelpMenu(context: Context) {
const config = context.config;
const startEnabled = config.commands.find((command) => command.name === "start");
const startDisabled = config.disabledCommands.some((command) => command === "start");
let helpMenu = "### Available Commands\n\n| Command | Description | Example |\n| --- | --- | --- |\n";
const commands = userCommands(config.miscellaneous.registerWalletWithVerification);

Expand All @@ -31,7 +31,7 @@ export function generateHelpMenu(context: Context) {
} |\n`) // add to help menu
);

if (!startEnabled) {
if (startDisabled) {
helpMenu += "\n\n***_To assign yourself to an issue, please open a draft pull request that is linked to it._***";
}
return helpMenu;
Expand Down
2 changes: 1 addition & 1 deletion src/handlers/pricing/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export async function handleParentIssue(context: Context, labels: Label[]) {
}

export function sortLabelsByValue(labels: Label[]) {
return labels.sort((a, b) => calculateLabelValue(a) - calculateLabelValue(b));
return labels.sort((a, b) => calculateLabelValue(a.name) - calculateLabelValue(b.name));
}

export function isParentIssue(body: string) {
Expand Down
5 changes: 1 addition & 4 deletions src/handlers/pricing/pre.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,13 @@ export async function syncPriceLabelsToConfig(context: Context) {

const {
features: { assistivePricing },
labels,
} = config;

if (!assistivePricing) {
logger.info(`Assistive pricing is disabled`);
return;
}

const timeLabels = labels.time.map((i) => i.name);
const priorityLabels = labels.priority.map((i) => i.name);
const aiLabels: string[] = [];
for (const timeLabel of config.labels.time) {
for (const priorityLabel of config.labels.priority) {
Expand All @@ -37,7 +34,7 @@ export async function syncPriceLabelsToConfig(context: Context) {
}
}

const pricingLabels: string[] = [...timeLabels, ...priorityLabels];
const pricingLabels: string[] = [...config.labels.time, ...config.labels.priority];

// List all the labels for a repository
const allLabels = await listLabelsForRepo(context);
Expand Down
5 changes: 3 additions & 2 deletions src/handlers/pricing/pricing-label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ export async function onLabelChangeSetPricing(context: Context): Promise<void> {
}

function getRecognizedLabels(labels: Label[], config: BotConfig) {
const isRecognizedLabel = (label: Label, labelConfig: { name: string }[]) =>
(typeof label === "string" || typeof label === "object") && labelConfig.some((item) => item.name === label.name);
const isRecognizedLabel = (label: Label, configLabels: string[]) =>
(typeof label === "string" || typeof label === "object") &&
configLabels.some((configLabel) => configLabel === label.name);

const recognizedTimeLabels: Label[] = labels.filter((label: Label) => isRecognizedLabel(label, config.labels.time));

Expand Down
4 changes: 2 additions & 2 deletions src/handlers/shared/pricing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ export function setPrice(context: Context, timeLabel: Label, priorityLabel: Labe

if (!timeLabel || !priorityLabel) throw logger.warn("Time or priority label is not defined");

const recognizedTimeLabels = labels.time.find((item) => item.name === timeLabel.name);
const recognizedTimeLabels = labels.time.find((configLabel) => configLabel === timeLabel.name);
if (!recognizedTimeLabels) throw logger.warn("Time label is not recognized");

const recognizedPriorityLabels = labels.priority.find((item) => item.name === priorityLabel.name);
const recognizedPriorityLabels = labels.priority.find((configLabel) => configLabel === priorityLabel.name);
if (!recognizedPriorityLabels) throw logger.warn("Priority label is not recognized");

const timeValue = calculateLabelValue(recognizedTimeLabels);
Expand Down
8 changes: 4 additions & 4 deletions src/handlers/wildcard/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@ export function taskPaymentMetaData(
} {
const { labels } = context.config;

const timeLabels = labels.time.filter((item) => issue.labels.map((i) => i.name).includes(item.name));
const priorityLabels = labels.priority.filter((item) => issue.labels.map((i) => i.name).includes(item.name));
const timeLabels = labels.time.filter((configLabel) => issue.labels.map((i) => i.name).includes(configLabel));
const priorityLabels = labels.priority.filter((configLabel) => issue.labels.map((i) => i.name).includes(configLabel));

const isTask = timeLabels.length > 0 && priorityLabels.length > 0;

const minTimeLabel =
timeLabels.length > 0
? timeLabels.reduce((a, b) => (calculateLabelValue(a) < calculateLabelValue(b) ? a : b)).name
? timeLabels.reduce((a, b) => (calculateLabelValue(a) < calculateLabelValue(b) ? a : b))
: null;

const minPriorityLabel =
priorityLabels.length > 0
? priorityLabels.reduce((a, b) => (calculateLabelValue(a) < calculateLabelValue(b) ? a : b)).name
? priorityLabels.reduce((a, b) => (calculateLabelValue(a) < calculateLabelValue(b) ? a : b))
: null;

const priceLabel = issue.labels.find((label) => label.name.includes("Price"))?.name || null;
Expand Down
18 changes: 9 additions & 9 deletions src/helpers/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ export function shouldSkip(context: ProbotContext) {
return response;
}

export function calculateLabelValue(label: { name: string }): number {
const matches = label.name.match(/\d+/);
export function calculateLabelValue(label: string): number {
const matches = label.match(/\d+/);
const number = matches && matches.length > 0 ? parseInt(matches[0]) || 0 : 0;
if (label.name.toLowerCase().includes("priority")) return number;
// throw new Error(`Label ${label.name} is not a priority label`);
if (label.name.toLowerCase().includes("minute")) return number * 0.002;
if (label.name.toLowerCase().includes("hour")) return number * 0.125;
if (label.name.toLowerCase().includes("day")) return 1 + (number - 1) * 0.25;
if (label.name.toLowerCase().includes("week")) return number + 1;
if (label.name.toLowerCase().includes("month")) return 5 + (number - 1) * 8;
if (label.toLowerCase().includes("priority")) return number;
// throw new Error(`Label ${label} is not a priority label`);
if (label.toLowerCase().includes("minute")) return number * 0.002;
if (label.toLowerCase().includes("hour")) return number * 0.125;
if (label.toLowerCase().includes("day")) return 1 + (number - 1) * 0.25;
if (label.toLowerCase().includes("week")) return number + 1;
if (label.toLowerCase().includes("month")) return 5 + (number - 1) * 8;
return 0;
}

Expand Down
32 changes: 10 additions & 22 deletions src/types/configuration-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,16 @@ const allHtmlElementsSetToZero = validHTMLElements.reduce((accumulator, current)
return accumulator;
}, {} as Record<keyof HTMLElementTagNameMap, number>);

const allCommands = userCommands(false).map((cmd) => ({ name: cmd.id.replace("/", ""), enabled: false }));
const allCommands = userCommands(false).map((cmd) => cmd.id.replace("/", ""));

const defaultTimeLabels = [
{ name: "Time: <1 Hour" },
{ name: "Time: <2 Hours" },
{ name: "Time: <4 Hours" },
{ name: "Time: <1 Day" },
{ name: "Time: <1 Week" },
];
const defaultTimeLabels = ["Time: <1 Hour", "Time: <2 Hours", "Time: <4 Hours", "Time: <1 Day", "Time: <1 Week"];

const defaultPriorityLabels = [
{ name: "Priority: 1 (Normal)" },
{ name: "Priority: 2 (Medium)" },
{ name: "Priority: 3 (High)" },
{ name: "Priority: 4 (Urgent)" },
{ name: "Priority: 5 (Emergency)" },
"Priority: 1 (Normal)",
"Priority: 2 (Medium)",
"Priority: 3 (High)",
"Priority: 4 (Urgent)",
"Priority: 5 (Emergency)",
];

function StrictObject<T extends TProperties>(obj: T, options?: ObjectOptions) {
Expand Down Expand Up @@ -101,13 +95,7 @@ export const BotConfigSchema = StrictObject(
basePriceMultiplier: T.Number({ default: 1 }),
issueCreatorMultiplier: T.Number({ default: 1 }),
}),
commands: T.Array(
StrictObject({
name: T.String(),
enabled: T.Boolean(),
}),
{ default: allCommands }
),
disabledCommands: T.Array(T.String(), { default: allCommands }),
Copy link
Owner

Choose a reason for hiding this comment

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

Perhaps it makes sense to enable all commands by default? It seems that disabling by default has always caused more headaches when starting new repos for us? rfc @whilefoo

incentives: StrictObject({
comment: StrictObject({
elements: T.Record(T.Union(HtmlEntities), T.Number({ default: 0 }), { default: allHtmlElementsSetToZero }),
Expand All @@ -121,8 +109,8 @@ export const BotConfigSchema = StrictObject(
}),
}),
labels: StrictObject({
time: T.Array(StrictObject({ name: T.String() }), { default: defaultTimeLabels }),
priority: T.Array(StrictObject({ name: T.String() }), { default: defaultPriorityLabels }),
time: T.Array(T.String(), { default: defaultTimeLabels }),
priority: T.Array(T.String(), { default: defaultPriorityLabels }),
}),
miscellaneous: StrictObject({
maxConcurrentTasks: T.Number({ default: Number.MAX_SAFE_INTEGER }),
Expand Down
53 changes: 11 additions & 42 deletions src/utils/generate-configuration.ts
Original file line number Diff line number Diff line change
@@ -1,75 +1,44 @@
import { Value } from "@sinclair/typebox/value";
import { DefinedError } from "ajv";
import merge from "lodash/merge";
import mergeWith from "lodash/merge";
import { Context as ProbotContext } from "probot";
import YAML from "yaml";
import Runtime from "../bindings/bot-runtime";
import { BotConfig, Payload, stringDuration, validateBotConfig } from "../types";
import ubiquibotConfigDefault from "../ubiquibot-config-default";

const UBIQUIBOT_CONFIG_REPOSITORY = "ubiquibot-config";
const UBIQUIBOT_CONFIG_FULL_PATH = ".github/ubiquibot-config.yml";

export async function generateConfiguration(context: ProbotContext): Promise<BotConfig> {
const payload = context.payload as Payload;

const organizationConfiguration = parseYaml(
const orgConfig = parseYaml(
await download({
context,
repository: UBIQUIBOT_CONFIG_REPOSITORY,
owner: payload.organization?.login || payload.repository.owner.login,
})
);

const repositoryConfiguration = parseYaml(
const repoConfig = parseYaml(
Comment on lines +15 to +23
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer that you do not shorten words. The reason why is to reduce cognitive overhead when reading through all of the code.

await download({
context,
repository: payload.repository.name,
owner: payload.repository.owner.login,
})
);

let orgConfig: BotConfig | undefined;
if (organizationConfiguration) {
const valid = validateBotConfig(organizationConfiguration);
if (!valid) {
let errMsg = getErrorMsg(validateBotConfig.errors as DefinedError[]);
if (errMsg) {
errMsg = `Invalid org configuration! \n${errMsg}`;
if (payload.issue?.number)
await context.octokit.issues.createComment({
owner: payload.repository.owner.login,
repo: payload.repository.name,
issue_number: payload.issue?.number,
body: errMsg,
});
throw new Error(errMsg);
const merged = mergeWith({}, orgConfig, repoConfig, (objValue: unknown, srcValue: unknown) => {
if (Array.isArray(objValue) && Array.isArray(srcValue)) {
// if it's string array, concat and remove duplicates
if (objValue.every((value) => typeof value === "string")) {
return [...new Set(objValue.concat(srcValue))];
}
// otherwise just concat
return objValue.concat(srcValue);
}
orgConfig = organizationConfiguration as BotConfig;
}

let repoConfig: BotConfig | undefined;
if (repositoryConfiguration) {
const valid = validateBotConfig(repositoryConfiguration);
if (!valid) {
let errMsg = getErrorMsg(validateBotConfig.errors as DefinedError[]);
if (errMsg) {
errMsg = `Invalid repo configuration! \n${errMsg}`;
if (payload.issue?.number)
await context.octokit.issues.createComment({
owner: payload.repository.owner.login,
repo: payload.repository.name,
issue_number: payload.issue?.number,
body: errMsg,
});
throw new Error(errMsg);
}
}
repoConfig = repositoryConfiguration as BotConfig;
}
});

const merged = merge(ubiquibotConfigDefault, orgConfig, repoConfig);
const valid = validateBotConfig(merged);
if (!valid) {
let errMsg = getErrorMsg(validateBotConfig.errors as DefinedError[]);
Expand Down Expand Up @@ -111,28 +80,28 @@
let errorMsg = "";
try {
config.timers.reviewDelayTolerance = Value.Decode(stringDuration(), config.timers.reviewDelayTolerance);
} catch (err: any) {

Check warning on line 83 in src/utils/generate-configuration.ts

View workflow job for this annotation

GitHub Actions / build

Unexpected any. Specify a different type
if (err.value) {
errorMsg += `Invalid reviewDelayTolerance value: ${err.value}\n`;
}
}
try {
config.timers.taskStaleTimeoutDuration = Value.Decode(stringDuration(), config.timers.taskStaleTimeoutDuration);
} catch (err: any) {

Check warning on line 90 in src/utils/generate-configuration.ts

View workflow job for this annotation

GitHub Actions / build

Unexpected any. Specify a different type
if (err.value) {
errorMsg += `Invalid taskStaleTimeoutDuration value: ${err.value}\n`;
}
}
try {
config.timers.taskFollowUpDuration = Value.Decode(stringDuration(), config.timers.taskFollowUpDuration);
} catch (err: any) {

Check warning on line 97 in src/utils/generate-configuration.ts

View workflow job for this annotation

GitHub Actions / build

Unexpected any. Specify a different type
if (err.value) {
errorMsg += `Invalid taskFollowUpDuration value: ${err.value}\n`;
}
}
try {
config.timers.taskDisqualifyDuration = Value.Decode(stringDuration(), config.timers.taskDisqualifyDuration);
} catch (err: any) {

Check warning on line 104 in src/utils/generate-configuration.ts

View workflow job for this annotation

GitHub Actions / build

Unexpected any. Specify a different type
if (err.value) {
errorMsg += `Invalid taskDisqualifyDuration value: ${err.value}\n`;
}
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8797,7 +8797,7 @@ zod-validation-error@^1.5.0:
resolved "https://registry.yarnpkg.com/zod-validation-error/-/zod-validation-error-1.5.0.tgz#2b355007a1c3b7fb04fa476bfad4e7b3fd5491e3"
integrity sha512-/7eFkAI4qV0tcxMBB/3+d2c1P6jzzZYdYSlBuAklzMuCrJu5bzJfHS0yVAS87dRHVlhftd6RFJDIvv03JgkSbw==

[email protected], zod@^3.22.4:
[email protected]:
version "3.22.4"
resolved "https://registry.yarnpkg.com/zod/-/zod-3.22.4.tgz#f31c3a9386f61b1f228af56faa9255e845cf3fff"
integrity sha512-iC+8Io04lddc+mVqQ9AZ7OQ2MrUKGN+oIQyq1vemgt46jwCwLfhq7/pwnBnNXXXZb8VTVLKwp9EDkx+ryxIWmg==
Loading