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

Add Issue Link Creation Feature in IssueModal Component #15

Merged
merged 15 commits into from
Dec 23, 2023

Conversation

claygorman
Copy link
Contributor

@claygorman claygorman commented Dec 23, 2023

Type

Enhancement


Description

  • Added the ability to create issue links directly in the IssueModal component.
  • Introduced new components LinkIssueSearch and LinkIssueTypeDropdown for selecting the issue to be linked and the type of link respectively.
  • Added new GraphQL queries and mutations for fetching issues based on a search query, and for creating and deleting issue links.
  • Updated the issues resolver to utilize the new vectorSearch field for more efficient and accurate search results.
  • Added new input types for creating and deleting issue links in the GraphQL schema.
  • Added a new vectorSearch field in the Issue model for improved full-text search.
  • Updated the Issue model to include a relationship with the Project model.

PR changes walkthrough

Relevant files                                                                                                                                 
Enhancement
5 files
IssueLinkedIssues.tsx                                                                             
    frontend/components/IssueModal/IssueLinkedIssues.tsx

    This file has been significantly updated to support the
    creation of issue links directly in the IssueModal
    component. It introduces new components such as
    LinkIssueSearch and LinkIssueTypeDropdown to facilitate
    the selection of the issue to be linked and the type of link
    respectively. It also includes the necessary GraphQL
    mutations to create and delete issue links. Additionally, it
    adds a CSS bug comment and optimizes the key prop in the
    IssueLinkedIssues component.

+429/-29
gql-queries-mutations.ts                                                                       
    frontend/gql/gql-queries-mutations.ts

    This file has been updated to include new GraphQL queries
    and mutations. The GET_ISSUES_QUERY is added to fetch
    issues based on a search query, and the
    CREATE_ISSUE_LINK_MUTATION and
    DELETE_ISSUE_LINK_MUTATION are added to create and delete
    issue links respectively. A comment about creating
    small/medium/large variants for the Button component is also
    added.

+45/-0
resolvers.js                                                                                               
    backend/src/resolvers.js

    This file has been updated to include new resolvers for
    creating and deleting issue links. It also modifies the
    issues resolver to utilize the new vectorSearch field
    for more efficient and accurate search results. The
    condition which checks if either projectId or id is not
    provided is removed.

+33/-7
type-defs.js                                                                                               
    backend/src/type-defs.js

    This file has been updated to include new input types for
    creating and deleting issue links. These input types are
    used in the new mutations added to the Mutation type.

+16/-0
issue.js                                                                                                       
    backend/src/db/models/issue.js

    This file has been updated to include a new vectorSearch
    field of the type TSVECTOR in the Issue model. This field is
    used for improved full-text search. The Issue model is also
    updated to include a relationship with the Project model.

+7/-1

The changes introduce a new feature allowing users to input a link connection between issues directly in the IssueModal component. Two dropdowns were added: one for defining the link type, and another for selecting the issue to be linked. The buttons for creating the link, cancelling the operation, or searching for an issue were also incorporated.
The main modification involves adding a vector_search column and corresponding index to the 'issues' table in sequelize migrations. This newly added TSVECTOR type column is used for improved full-text search. Meanwhile, the resolvers.js file has been updated to use the exact match operator for the 'id' search field and case-insensitive like for the 'title' field.
The field 'vectorSearch' of the type TSVECTOR has been added to the Issue model in the database. The search functionality in resolvers.js has also been updated to utilize this new field for more efficient and accurate search results. The commit removes the redundant condition which checks if either projectId or id is not provided.
The code has been simplified by switching to async/await pattern from promises for better readability and ease of understanding. This was done in the `up` and `down` functions within the `add-issue-vector-search` migrations script. Unnecessary console logs have also been removed.
This update includes the addition of 'project' details to the 'Issue' GraphQL type, enabling these details to be accessed with the issue. Additionally, the "LinkIssueSearch" function in 'IssueLinkedIssues.tsx' has been enhanced to actually perform a query and return matching issues. To improve performance, a debounce function has been implemented to delay the search trigger by 300ms.
A comment has been added to the IssueLinkedIssues file in the frontend components to highlight an existing CSS bug. The bug is causing a problem with how the dropdown appears in a modal. This comment will serve as a reminder to fix the issue in the future.
The update simplifies the mapped Listbox.Option elements in the IssueLinkedIssues component by directly using 'link.value' as the key prop instead of 'linkIdx'. This can potentially optimize react re-renderings for this component.
This commit introduces methods for creating and deleting issue links. These new functionalities are introduced in the backend and connected to the frontend via GraphQL queries and mutations. Changes also include type definitions and model adjustments for proper link handling.
A delete issue link mutation has been implemented on both the frontend and backend. On the frontend, it's represented as a 'X' icon in the issue links section. On the backend, it's handled by destroying the corresponding record in the IssueLinks model in the database.
A new type named 'IssueLink' has been defined to accommodate link data including 'issueId', 'linkedIssueId', and 'linkType'. Two new functions have been added: 'handleDeleteIssueLink' and 'handleCreateIssueLink' for issue link handling on deletion and creation events respectively. In these functions, mutation calls for creating and deleting issue links have been abstracted.
A new feature is implemented to enable opening of linked issues from the issue modal list. A function named 'handleOpenIssue' is added which takes an issue id and navigates to the respective issue detail page. Click actions are also added to the issue id and title list items to trigger this function.
Copy link
Contributor

github-actions bot commented Dec 23, 2023

PR Analysis

(review updated until commit 4c99cdc)

  • 🎯 Main theme: Adding issue link creation feature in IssueModal component
  • 📝 PR summary: This PR introduces the ability to create issue links directly in the IssueModal component. It adds new components for selecting the issue to be linked and the type of link, and updates the GraphQL schema and resolvers to support these new features. It also enhances the search functionality by adding a vectorSearch field to the Issue model.
  • 📌 Type of PR: Enhancement
  • ⏱️ Estimated effort to review [1-5]: 4, because the PR involves changes in multiple areas including frontend components, GraphQL schema, and database models. It also introduces new features that require careful review to ensure they are implemented correctly.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR is well-structured and the changes are logically grouped. The use of GraphQL for fetching and manipulating data is a good practice. However, there are a few areas that could be improved. For instance, the error handling could be more robust, and the code could be more modular by separating large functions into smaller ones.

  • 🤖 Code feedback:
    relevant filefrontend/components/IssueModal/IssueLinkedIssues/index.tsx
    suggestion      Consider breaking down the large component into smaller ones to improve readability and maintainability. [medium]
    relevant line'+import { usePathname, useRouter, useSearchParams } from 'next/navigation;'

    relevant filefrontend/components/IssueModal/IssueLinkedIssues/index.tsx
    suggestion      Add error handling for the GraphQL queries and mutations. This will help to catch and handle any errors that may occur during the execution of these operations. [important]
    relevant line'+const { data, error, loading } = useQuery(GET_ISSUE_QUERY, {'

    relevant filefrontend/gql/gql-queries-mutations.ts
    suggestion      Consider using a more descriptive name for the mutation. For example, instead of 'CREATE_ISSUE_LINK_MUTATION', you could use 'CREATE_ISSUE_LINK_REQUEST'. This would make it clearer that this is a request to create a link. [medium]
    relevant line'+export const CREATE_ISSUE_LINK_MUTATION = gql(/* GraphQL */ `'

    relevant filebackend/src/db/models/issue-links.js
    suggestion      The 'linkTypeInverted' field is defined as a VIRTUAL field but its setter function is empty. If this field is not meant to be set, consider removing the setter function. [medium]
    relevant line'+ set(value) {},'

How to use

Instructions

To invoke the PR-Agent, add a comment using one of the following commands:
/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs: Generate docstring for new components introduced in the PR.
/generate_labels: Generate labels for the PR based on the PR's contents.
see the tools guide for more details.

To edit any configuration parameter from the configuration.toml, add --config_path=new_value.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, add a /config comment.

@claygorman
Copy link
Contributor Author

/describe

@github-actions github-actions bot changed the title issue-link-create Add Issue Link Creation Feature in IssueModal Component Dec 23, 2023
@github-actions github-actions bot added the enhancement New feature or request label Dec 23, 2023
Copy link
Contributor

PR Description updated to latest commit (88e6b48)

The codebase has been refactored to split the complex IssueLinkedIssues component into separate, more manageable elements. This change improves readability and maintainability, as each subcomponent can now be understood and manipulated independently. The subcomponents were moved into their respective files: LinkIssueTypeDropdown, LinkIssueSearch, and the Index file for the IssueLinkedIssues.
The code in the LinkIssueSearch component has been refactored for improved readability and efficiency. The sequence of useState and useLazyQuery hooks have been reordered, creating a logical, easy-to-follow flow within the component. The code also optimizes performance by implementing a debounce function on handleChange for efficient search results.
@claygorman
Copy link
Contributor Author

/review

Copy link
Contributor

Persistent review updated to latest commit 4c99cdc

The unused setter function for the "linkTypeInverted" field in the issue-links model has been removed. This cleanup further simplifies the model and enhances code readability.
@claygorman
Copy link
Contributor Author

/improve

The code now includes the DeleteIssueLink mutation in GraphQL schema. The new feature enables us to remove the issue links in the application. The respective GraphQL documents have been generated and added to the schema.
Comment on lines 198 to 199
export function gql(source: string) {
return (documents as any)[source] ?? {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Avoid using any type in TypeScript. It's better to provide a specific type or use a generic if the type is not known.

Suggested change
export function gql(source: string) {
return (documents as any)[source] ?? {};
export function gql(source: string) {
return (documents as Record<string, unknown>)[source] ?? {};
}

Comment on lines +10 to +24
const LinkIssueSearch = ({
stateCallback,
}: {
stateCallback?: (args: any) => void;
}) => {
const [selected, setSelected] = useState({});
const [query, setQuery] = useState('');
const [searchIssues, { data, error, loading }] = useLazyQuery(
GET_ISSUES_QUERY,
{
fetchPolicy: 'network-only',
}
);
const handleChange = (e: any) => {
setQuery(e.target.value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Use a more specific type than 'any' for the 'stateCallback' function parameter and 'e' parameter in 'handleChange' function.

Suggested change
const LinkIssueSearch = ({
stateCallback,
}: {
stateCallback?: (args: any) => void;
}) => {
const [selected, setSelected] = useState({});
const [query, setQuery] = useState('');
const [searchIssues, { data, error, loading }] = useLazyQuery(
GET_ISSUES_QUERY,
{
fetchPolicy: 'network-only',
}
);
const handleChange = (e: any) => {
setQuery(e.target.value);
interface StateCallbackArgs {
selectedIssue: Record<string, unknown>;
}
const LinkIssueSearch = ({
stateCallback,
}: {
stateCallback?: (args: StateCallbackArgs) => void;
}) => {
...
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
setQuery(e.target.value);
};
...
};

Comment on lines +4 to 7
// TODO: make small/medium/large variants
export const Button = ({
text,
variant,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Add a TODO comment to remind of the need to implement button size variants.

Suggested change
// TODO: make small/medium/large variants
export const Button = ({
text,
variant,
// TODO: Implement small/medium/large variants for the button
export const Button = ({
text,
variant,
...
};

Comment on lines 38 to 42
linkTypeInverted: {
type: DataTypes.STRING,
type: DataTypes.VIRTUAL,
get() {
return inverseLinkType?.[this.getDataValue('linkType')];
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: It's better to use DataTypes.STRING for 'linkTypeInverted' as it's a string type.

Suggested change
linkTypeInverted: {
type: DataTypes.STRING,
type: DataTypes.VIRTUAL,
get() {
return inverseLinkType?.[this.getDataValue('linkType')];
},
linkTypeInverted: {
type: DataTypes.STRING,
get() {
return inverseLinkType?.[this.getDataValue('linkType')];
},
},

@claygorman claygorman merged commit 00566d9 into master Dec 23, 2023
1 of 2 checks passed
@claygorman claygorman deleted the issue-link-create branch December 23, 2023 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant