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

feat(sprint-poker): Filter GitLab issues by project #6323

Merged

Conversation

nickoferrall
Copy link
Contributor

@nickoferrall nickoferrall commented Mar 31, 2022

Fix: #6046

Note: there is a bug as mentioned in the comment below, but the PR is accomplishing the AC of being able to filter issues by projects, so I think it's safe to move this to Reviewer Review.

@nickoferrall nickoferrall changed the base branch from feat/6045/search-gitlab-issues to master March 31, 2022 15:55
@nickoferrall nickoferrall changed the base branch from master to feat/6045/search-gitlab-issues March 31, 2022 16:03
@nickoferrall
Copy link
Contributor Author

Hey @mattkrick, the problem we discussed yesterday during our 1-1 is still causing trouble. After selecting one of the project filters in GitLabScopingSearchFilterMenu, the GitLabScopingSearchResults query successfully filters by the selected project. However, the GitLabScopingSearchFilterMenu query also filters even though no ids have been passed as an arg.

projects-filter-bug

In the gif above, I'm using a network-only fetch policy. If I use a store-or-network policy, unselecting the selected project filter does not result in all menu options returning to the menu, i.e. there's still only one project in the menu.

I suspect the GitLab queries are being tangled up when they're stitched together. Georg came across a similar problem when reviewing one of my other PRs: #6321

@@ -126,7 +129,6 @@ const GitLabScopingSearchResults = (props: Props) => {
query
)

const lastItem = useLoadNextOnScrollBottom(paginationRes, {}, 20)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Scrolling to the bottom of the page when the projects query contains a projectsIds causes an infinite loop. I spent time digging into this, but as we're refactoring this query in #6325 and as the user can still find the desired issue using the search & filter menu, I think it's safe to remove for now.

label={
<StyledMenuItemLabel>
<StyledCheckBox active={isSelected} />
<TypeAheadLabel query={searchQuery} label={fullPath} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In GitLab, a project can have subprojects. In the Parabol1 repo, we have several projects that contain several subprojects. So, if we show the name of the project in the filter menu, there can be several projects that have the same name. In Parabol1, we have many subprojects called ppmi, for example. To differentiate between projects, I think it's best to use fullPath instead of name in the menu.

@mattkrick
Copy link
Member

to fix the search thing, just use search: "" on the search filter args list to tell relay it's a totally different query.
loom: https://www.loom.com/share/33c450c0c9394aab8765f6d0ce009636

… in project connection so we can add issues with a filter
@nickoferrall nickoferrall changed the base branch from feat/6045/search-gitlab-issues to master April 1, 2022 11:10
@nickoferrall nickoferrall changed the base branch from master to feat/6045/search-gitlab-issues April 1, 2022 11:10
@nickoferrall nickoferrall linked an issue Apr 4, 2022 that may be closed by this pull request
6 tasks
@@ -53,7 +56,7 @@ const GitLabScopingSearchResults = (props: Props) => {
graphql`
fragment GitLabScopingSearchResults_query on Query
@argumentDefinitions(
projectsFirst: {type: "Int", defaultValue: 20}
projectsFirst: {type: "Int", defaultValue: 75}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it safe to increase to that number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this discussion, we decided to have a large initial query where we grab many projects and their issues. We could reduce the default value to 50 if 75 feels too large?

This will ultimately get refactored in this issue: #6325

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I just remember we had this issue. If we have refactor issue, I think it is fine

Copy link
Contributor

@igorlesnenko igorlesnenko left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just a couple of things.

  • We removed gitlabTypes.ts from Git but here we seem to be bringing it back
  • While testing various GitLab issues, I noticed that sometimes I need to reconnect GitLab. Usually, I open a list of issues and see it is empty but no error is displayed. Reconnecting GitLab fixes the issue for me. Feels like at some point GitLab token expires and we do not handle it. Is it a known issue?

@nickoferrall
Copy link
Contributor Author

Great, thanks for the review. I've removed the GitLab types. I've also noticed GitLab needing to be reintegrated occasionally so I've created an issue to dig into this: #6343

@igorlesnenko igorlesnenko requested a review from mattkrick April 5, 2022 14:36
Copy link
Member

@mattkrick mattkrick left a comment

Choose a reason for hiding this comment

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

-2 just 1 fatal edge case, everything else is pretty minor. great job!

@@ -26,6 +26,10 @@ const StyledCheckBox = styled(Checkbox)({
})
const StyledMenuItemLabel = styled(MenuItemLabel)({})

const StyledMenu = styled(Menu)({
maxWidth: '100%'
Copy link
Member

Choose a reason for hiding this comment

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

+1 i tried commenting this out & it looked the same. any steps to reproduce the problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a project fullPath is particularly long, it gets truncated without setting maxWidth to 100%.
Without maxWidth: '100%':
before

With maxWidth: '100%':
after

const {queryString, selectedProjectsIds} = gitlabSearchQuery!
const normalizedQueryString = queryString.trim()
const normalizedSelectedProjectsIds = selectedProjectsIds?.length
? (selectedProjectsIds as string[])
Copy link
Member

Choose a reason for hiding this comment

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

+1 can remove casting, the end result can still be null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we have to keep the casting here because if we remove it, TS complains that normalizedSelectedProjectsIds can be readonly

@@ -70,8 +70,7 @@ const ScopePhaseArea = (props: Props) => {
const {meeting} = props
const isDesktop = useBreakpoint(Breakpoint.SIDEBAR_LEFT)
const {viewerMeetingMember} = meeting
if (!viewerMeetingMember) return null
const {user, teamMember} = viewerMeetingMember
const {user, teamMember} = viewerMeetingMember!
Copy link
Member

Choose a reason for hiding this comment

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

-2 what if a user visits a meeting that they didn't join while it was active?
if this is about a lint rule you can just add a comment to ignore that rule for this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I was just trying to remove the lint rule. Change reverted 👍

packages/server/graphql/types/GitLabSearchQuery.ts Outdated Show resolved Hide resolved
@nickoferrall nickoferrall requested a review from mattkrick April 7, 2022 11:47
@mattkrick
Copy link
Member

LGTM!

@mattkrick mattkrick merged commit 86f3b43 into feat/6045/search-gitlab-issues Apr 19, 2022
@mattkrick mattkrick deleted the feat/6046/project-filter-gitlab branch April 19, 2022 00:46
mattkrick added a commit that referenced this pull request Apr 19, 2022
* render list of gitlab issues

* use usePaginationFragment

* increase default value for usePaginationFragment

* implement GitLabScopingSearchResultItem

* add GitLabScopingSelectAllIssues

* can add and remove gitlab issues in scope phase

* add UpdatePokerScopeMutation gitlab optimistic updater

* able to select all issues

* clean up type errors

* add projects name alias to rootSchema

* include projectIds in search query

* return iid from GitLabIssueId

* sort projects by lastActivityAt

* implement fetchGitLabProjects

* implement gitlab issue menu

* implement NewGitLabIssueMenuRoot instead of using useAllIntegrations

* load next if projects dont have any issues

* update comment

* remove gitlab menu root and use defaultProjects query to populate menu

* increase projectsFirst from 10 to 20

* query all gitlab projects

* chore(comment): how to extend BaseTaskIntegration

* able to create a new gitlab issue

* use GitLabServerManager and implement parseWebPath

* fix undefined baseUri

* changed taskIntegrationGitLab and GitLabId to use providerId

* lowercase gitlabRequest to be consistent with gh

* add handle create gitlab issue

* add info fragment and return data instead of data.issue

* add serverBaseUrl

* adjusted root schema and can now render projects in input menu again

* clean up update poker scope and create task gitlab

* fix ts errors

* add gitlab query types

* add fullPath to gitLab issue edge if exists

* get nodes appearing on insert

* get gitlab issue title in create task updater

* include webUrl in createTask query so user can click on newly create issue url

* add first and sort to allProjects query

* rename GitLabRepo to GitLabProject

* pass meetingId to issue input rather than querying it

* add gitlab search query

* filter by gitlab search query

* refactor to hooks

* remove __typename and resolveTypes

* update UpdatePokerScopeMutation to fix selectAll bug

* add search icon

* remove searchQuery from scoping results query

* clean-up return statement in fetchGitLabProjects

* add search to issue args

* include search string when adding new gitlab issue

* refactor search query to a gql object

* merge with master

* use react-swipeable-views workaround

* add _xGitLabProject resolver

* merge with master

* remove resolverTypes and gitlabTypes

* map over tabs instead of contents

* fix(gitlab): add proper client-side alias handling (#6361)

* resolve to aliased fields

* resolve to aliased fields

* make poker input menu a dropdown and fix width

* refactor baseTabs to include Component

* create a single source of truth for gitlab issue args

* remove refetchable from gitlab scoping results query

* feat(sprint-poker): Filter GitLab issues by project (#6323)

* query projects from project filter menu

* render projects in filter menu

* use fullPath in gitlab menu and adjust max width

* refactor gitlab search query from string to object with projectIds

* selecting a project in the filter menu adds the item to selectedProjectsFullPath

* selecting a project filters the results

* add search icon

* remove alias

* make selectedProjectIds nullable

* remove useLoadNextOnScrollBottom and increase default projects first

* fix selectedProjects type err

* add search string to differentiate project menu query and include ids in project connection so we can add issues with a filter

* remove gitlab types

* add viewerMeetingMember check and remove selectedProjectsIds resolver

* make selectedProjectsIds null if empty array

Co-authored-by: Matt Krick <[email protected]>

Co-authored-by: Matt Krick <[email protected]>
JimmyLv pushed a commit that referenced this pull request Apr 19, 2022
* render list of gitlab issues

* use usePaginationFragment

* increase default value for usePaginationFragment

* implement GitLabScopingSearchResultItem

* add GitLabScopingSelectAllIssues

* can add and remove gitlab issues in scope phase

* add UpdatePokerScopeMutation gitlab optimistic updater

* able to select all issues

* clean up type errors

* add projects name alias to rootSchema

* include projectIds in search query

* return iid from GitLabIssueId

* sort projects by lastActivityAt

* implement fetchGitLabProjects

* implement gitlab issue menu

* implement NewGitLabIssueMenuRoot instead of using useAllIntegrations

* load next if projects dont have any issues

* update comment

* remove gitlab menu root and use defaultProjects query to populate menu

* increase projectsFirst from 10 to 20

* query all gitlab projects

* chore(comment): how to extend BaseTaskIntegration

* able to create a new gitlab issue

* use GitLabServerManager and implement parseWebPath

* fix undefined baseUri

* changed taskIntegrationGitLab and GitLabId to use providerId

* lowercase gitlabRequest to be consistent with gh

* add handle create gitlab issue

* add info fragment and return data instead of data.issue

* add serverBaseUrl

* adjusted root schema and can now render projects in input menu again

* clean up update poker scope and create task gitlab

* fix ts errors

* add gitlab query types

* add fullPath to gitLab issue edge if exists

* get nodes appearing on insert

* get gitlab issue title in create task updater

* include webUrl in createTask query so user can click on newly create issue url

* add first and sort to allProjects query

* rename GitLabRepo to GitLabProject

* pass meetingId to issue input rather than querying it

* add gitlab search query

* filter by gitlab search query

* refactor to hooks

* remove __typename and resolveTypes

* update UpdatePokerScopeMutation to fix selectAll bug

* add search icon

* remove searchQuery from scoping results query

* clean-up return statement in fetchGitLabProjects

* add search to issue args

* include search string when adding new gitlab issue

* refactor search query to a gql object

* merge with master

* use react-swipeable-views workaround

* add _xGitLabProject resolver

* merge with master

* remove resolverTypes and gitlabTypes

* map over tabs instead of contents

* fix(gitlab): add proper client-side alias handling (#6361)

* resolve to aliased fields

* resolve to aliased fields

* make poker input menu a dropdown and fix width

* refactor baseTabs to include Component

* create a single source of truth for gitlab issue args

* remove refetchable from gitlab scoping results query

* feat(sprint-poker): Filter GitLab issues by project (#6323)

* query projects from project filter menu

* render projects in filter menu

* use fullPath in gitlab menu and adjust max width

* refactor gitlab search query from string to object with projectIds

* selecting a project in the filter menu adds the item to selectedProjectsFullPath

* selecting a project filters the results

* add search icon

* remove alias

* make selectedProjectIds nullable

* remove useLoadNextOnScrollBottom and increase default projects first

* fix selectedProjects type err

* add search string to differentiate project menu query and include ids in project connection so we can add issues with a filter

* remove gitlab types

* add viewerMeetingMember check and remove selectedProjectsIds resolver

* make selectedProjectsIds null if empty array

Co-authored-by: Matt Krick <[email protected]>

Co-authored-by: Matt Krick <[email protected]>
atannus pushed a commit to atannus/parabol that referenced this pull request Apr 26, 2022
* render list of gitlab issues

* use usePaginationFragment

* increase default value for usePaginationFragment

* implement GitLabScopingSearchResultItem

* add GitLabScopingSelectAllIssues

* can add and remove gitlab issues in scope phase

* add UpdatePokerScopeMutation gitlab optimistic updater

* able to select all issues

* clean up type errors

* add projects name alias to rootSchema

* include projectIds in search query

* return iid from GitLabIssueId

* sort projects by lastActivityAt

* implement fetchGitLabProjects

* implement gitlab issue menu

* implement NewGitLabIssueMenuRoot instead of using useAllIntegrations

* load next if projects dont have any issues

* update comment

* remove gitlab menu root and use defaultProjects query to populate menu

* increase projectsFirst from 10 to 20

* query all gitlab projects

* chore(comment): how to extend BaseTaskIntegration

* able to create a new gitlab issue

* use GitLabServerManager and implement parseWebPath

* fix undefined baseUri

* changed taskIntegrationGitLab and GitLabId to use providerId

* lowercase gitlabRequest to be consistent with gh

* add handle create gitlab issue

* add info fragment and return data instead of data.issue

* add serverBaseUrl

* adjusted root schema and can now render projects in input menu again

* clean up update poker scope and create task gitlab

* fix ts errors

* add gitlab query types

* add fullPath to gitLab issue edge if exists

* get nodes appearing on insert

* get gitlab issue title in create task updater

* include webUrl in createTask query so user can click on newly create issue url

* add first and sort to allProjects query

* rename GitLabRepo to GitLabProject

* pass meetingId to issue input rather than querying it

* add gitlab search query

* filter by gitlab search query

* refactor to hooks

* remove __typename and resolveTypes

* update UpdatePokerScopeMutation to fix selectAll bug

* add search icon

* remove searchQuery from scoping results query

* clean-up return statement in fetchGitLabProjects

* add search to issue args

* include search string when adding new gitlab issue

* refactor search query to a gql object

* merge with master

* use react-swipeable-views workaround

* add _xGitLabProject resolver

* merge with master

* remove resolverTypes and gitlabTypes

* map over tabs instead of contents

* fix(gitlab): add proper client-side alias handling (ParabolInc#6361)

* resolve to aliased fields

* resolve to aliased fields

* make poker input menu a dropdown and fix width

* refactor baseTabs to include Component

* create a single source of truth for gitlab issue args

* remove refetchable from gitlab scoping results query

* feat(sprint-poker): Filter GitLab issues by project (ParabolInc#6323)

* query projects from project filter menu

* render projects in filter menu

* use fullPath in gitlab menu and adjust max width

* refactor gitlab search query from string to object with projectIds

* selecting a project in the filter menu adds the item to selectedProjectsFullPath

* selecting a project filters the results

* add search icon

* remove alias

* make selectedProjectIds nullable

* remove useLoadNextOnScrollBottom and increase default projects first

* fix selectedProjects type err

* add search string to differentiate project menu query and include ids in project connection so we can add issues with a filter

* remove gitlab types

* add viewerMeetingMember check and remove selectedProjectsIds resolver

* make selectedProjectsIds null if empty array

Co-authored-by: Matt Krick <[email protected]>

Co-authored-by: Matt Krick <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sprint Poker: GitLab v1: Project filter menu
3 participants