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 Visibility Field to Projects #16

Merged
merged 1 commit into from
Dec 23, 2023
Merged

Conversation

claygorman
Copy link
Contributor

@claygorman claygorman commented Dec 23, 2023

Type

Enhancement


Description

This PR introduces a new visibility field for projects, with possible values of 'PUBLIC', 'INTERNAL', and 'PRIVATE'. The main changes include:

  • Updating the GraphQL queries, mutations, and fragments related to projects to include the visibility field.
  • Updating the project list in the frontend to display the visibility of each project.
  • Updating the createProject resolver to accept a visibility field in the input and include it when creating a new project in the database.
  • Adding a new ProjectVisibility enum to the GraphQL schema and including a visibility field in the Project and CreateProjectInput types.
  • Adding a new database migration to include a visibility column in the projects table.
  • Updating the Project model to include a visibility field, with a default value of 'internal' and a validation to ensure the value is one of 'PRIVATE', 'INTERNAL', or 'PUBLIC'.

PR changes walkthrough

Relevant files                                                                                                                                 
Enhancement
7 files
gql.ts                                                                                                           
    frontend/gql/__generated__/gql.ts

    The GraphQL queries and mutations related to projects have
    been updated to include the new visibility field. The
    ProjectFields and ProjectOnlyFields fragments now
    include visibility. The CreateProject mutation and
    GetProjects query now use the ProjectOnlyFields
    fragment.

+8/-8
gql-queries-mutations.ts                                                                       
    frontend/gql/gql-queries-mutations.ts

    The PROJECT_FIELDS and PROJECT_ONLY_FIELDS constants
    have been updated to include the visibility field. The
    CREATE_PROJECT_MUTATION and GET_PROJECTS queries now use
    the ProjectOnlyFields fragment.

+4/-8
index.tsx                                                                                                     
    frontend/components/ProjectList/index.tsx

    The project list now includes a column for visibility. The
    cols constant and the project list rendering have been
    updated to include this new column.

+12/-1
resolvers.js                                                                                               
    backend/src/resolvers.js

    The createProject resolver now accepts a visibility
    field in the input, with a default value of 'INTERNAL'. The
    visibility field is included when creating a new project
    in the database.

+10/-20
type-defs.js                                                                                               
    backend/src/type-defs.js

    A new ProjectVisibility enum has been added to the GraphQL
    schema, with possible values of 'PUBLIC', 'INTERNAL', and
    'PRIVATE'. The Project and CreateProjectInput types now
    include a visibility field of type ProjectVisibility.

+8/-0
20231223000001-add-project-visibility.js                                       
    backend/src/db/migrations/20231223000001-add-project-visibility.js

    A new database migration has been added to include a
    visibility column in the projects table.

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

    The Project model now includes a visibility field, with
    a default value of 'internal' and a validation to ensure the
    value is one of 'PRIVATE', 'INTERNAL', or 'PUBLIC'.

+7/-0

This commit introduces visibility setting for projects. It modifies the resolver to make visibility an optional input on project creation with a default set to 'INTERNAL'. It also adjusts the GraphQL schema to recognize the new visibility options and associates it with project entities.
@claygorman
Copy link
Contributor Author

/describe

Copy link
Contributor

PR Analysis

  • 🎯 Main theme: Adding a visibility feature to projects
  • 📝 PR summary: This PR introduces a visibility setting for projects. It modifies the GraphQL schema to recognize the new visibility options and associates it with project entities. The visibility setting can be 'PUBLIC', 'INTERNAL', or 'PRIVATE', with a default set to 'INTERNAL'.
  • 📌 Type of PR: Enhancement
  • ⏱️ Estimated effort to review [1-5]: 3, because the PR involves changes in multiple files, including frontend and backend, and requires understanding of GraphQL and Sequelize.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR is generally well-structured and follows good coding practices. The use of GraphQL fragments to reduce redundancy is commendable. However, it would be beneficial to add some tests to ensure the new visibility feature works as expected.

  • 🤖 Code feedback:
    relevant filebackend/src/resolvers.js
    suggestion      Consider using Sequelize transactions to ensure atomicity when creating a project and its associated issue statuses. This will prevent partial data creation in case of any failure. [important]
    relevant line"+ // TODO: we should do a sequelize transaction here"

    relevant filebackend/src/db/models/project.js
    suggestion      It would be better to use an ENUM data type for the 'visibility' field in the Sequelize model. This will enforce the allowed values at the database level. [medium]
    relevant line"+ visibility: {"

    relevant filefrontend/components/ProjectList/index.tsx
    suggestion      For better readability and maintainability, consider creating a separate component for the table row. This will make the code cleaner and more modular. [medium]
    relevant line"+ "

    relevant filebackend/src/resolvers.js
    suggestion      Consider using a map function to create the issue statuses. This will make the code cleaner and easier to read. [medium]
    relevant line"+ const issueStatuses = await db.sequelize.models.IssueStatuses.bulkCreate("

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.

@github-actions github-actions bot changed the title add-project-visibility Add Visibility Field to Projects 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 (9a72543)

@claygorman claygorman merged commit 7aaa644 into master Dec 23, 2023
2 checks passed
@claygorman claygorman deleted the add-project-visibility branch December 23, 2023 22:18
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