Show total number of votes and maximum number of votes for all users #488

kimsey0 · 2022-12-09T11:47:23Z

Pull Request

Description

Added total number of votes cast by all participants plus max number of votes that can be cast by all participants to the Vote phase.

I did not update the screenshot in the README, since I can't recreate the user setup.

vsts-extension-retrospectives/README.md

Line 218 in c420655

    
           user show a "Your Votes: 1" in the feedback card.](https://github.com/microsoft/vsts-extension-retrospectives/raw/main/documentation/images/desktop/vote-feedback-in-progress.png)

I calculate these numbers in the render method, since this avoids duplicating logic across refreshFeedbackItems and onVoteCasted. Alternatively, the logic can be split up across these two places, the logic in onVoteCasted that calculates currentVoteCount could be moved out into the render method, or all of it could be extracted into a new method. Let me know what you prefer.

There is currently no tests for feedbackBoard, but I'm happy to add some that test this logic. Again, let me know.

Lastly, I simplified some lines using the Optional Chaining operator (?.) from TypeScript 3.7 and the null == undefined equality.

Bug or Feature?

Bug fix
New feature

Fundamentals

I have read the CONTRIBUTING document.
My code follows the code style of this project.
I have updated any relevant documentation accordingly.
I have included an update blurb (50 words or less) at the top of CHANGELOG.md,
to be included with the next release.
- I have included a link to this PR in that blurb.
I have added tests to cover my changes.
All new and existing tests passed.

Accessibility

I have tested my changes on both light and dark themes.
I have tested my changes on both mobile and desktop views, when needed.
I have included image descriptions and aria-labels where I can.

Screenshots and Logs

Before

After:

…or all users.

mindlessroman · 2022-12-12T19:35:55Z

!!! Thank you so much for the PR! I'll try to look at it as soon as possible, @kimsey0 ! We can also tweak the documentation/images to get that updated to match your work!

As for tests, if you are up for writing some, that'd be great. There should be some basic ones in the repo that you can use as a jumping off point.

polatengin · 2022-12-13T21:30:51Z

I liked the idea and implementation @kimsey0 🥳 if you can address comments of @mindlessroman, I'd like to merge this PR asap 😄 if you need help with writing tests, etc. please let us know, we can help you out 👍

a new exception if the previous one doesn't have a serverError property

…t to array

kimsey0 · 2023-02-03T12:54:40Z

Sorry for the long delay here.

I've added the skeleton for a test of the vote count display, but I'm having trouble with mocking all the different services the feedback board component uses. Specifically, I can't find any tests that mock the retrospective extension backend, and I don't know how you'd like it structured. I think properly testing this is a bit too big for me.

I don't know if one of you would like to take over from the skeleton (I've allowed edits by maintainers), if it can merge as-is (with or without the test changes), or if we should abandon this pull request?

kimsey0 · 2023-02-03T12:57:59Z

RetrospectiveExtension.Frontend/components/__tests__/feedbackBoard.test.tsx

+
+jest.mock('../feedbackBoardMetadataForm', () => mocked({}));
+jest.mock('azure-devops-extension-api/Work/WorkClient', () => {
+  const getTeamIterationsMock = () => {


These mocks are copied from feedbackBoardContainer.test.tsx. I don't know if they're general enough to be moved to the __mocks__ folder. It might also be possible to use a simplified version in these tests.

If this chunk is a copy of the other, I think that's a valid thing to move to __mocks__ folder so there's less duplicate code. It's probably okay to remove any bit that's not explicitly called, but that may be worth doing a local test or two to be sure.

kimsey0 · 2023-02-03T12:58:20Z

RetrospectiveExtension.Frontend/components/__mocks__/mocked_components/mockedFeedbackColumn.tsx

@@ -90,7 +89,7 @@ testColumnsObj[testColumnUuidOne] = {
 testColumnsObj[testColumnUuidTwo] = {
  columnProperties:
  {
-    id: TooltipOverflowMode,


I don't know why this was using this random import, but it seemed like a bug.

Appreciate the diligence! Always appreciated :D

kimsey0 · 2023-02-03T12:59:48Z

RetrospectiveExtension.Frontend/components/__tests__/feedbackBoard.test.tsx

+}));
+
+jest.mock('azure-devops-extension-api/Common', () => ({
+  getClient: (clientClass: typeof Object) => new clientClass(),


I think this could be moved into __mocks__/azure-devops-extension-api/Common/Common.tsx, but there are some of the other tests that depend on the current behavior.

I'll make a more overarching issue to maybe get the tests and components tidied eventually - may be a bit of an undertaking. For now, you should be good to go!

mindlessroman · 2023-02-03T15:32:24Z

👏 👏 👏 👏 👏 This is such a good start - I'll try to take a look this weekend!

kimsey0 · 2023-04-27T11:22:38Z

@mindlessroman, any news on this? 😄

mindlessroman · 2023-05-02T20:04:45Z

@mindlessroman, any news on this? 😄

Thank you for the patience here, the 9-5 stuff was occupying a lot of time. Will try to get this looked at as soon as I can.

mindlessroman · 2023-05-02T20:18:13Z

CHANGELOG.md

@@ -4,6 +4,10 @@ You can find the changelog of the Retrospective Extension below.

 _PS: Unfortunately, changelog before v1.0.46 is not available_ 🤦‍♂️

+## v1.90.X
+
+* Total number of team votes is now shown together with your own vote count. From [GitHub PR #488](https://github.com/microsoft/vsts-extension-retrospectives/pull/488).


mindlessroman · 2023-05-02T20:19:23Z

RetrospectiveExtension.Frontend/components/feedbackBoard.tsx

@@ -69,7 +69,7 @@ class FeedbackBoard extends React.Component<FeedbackBoardProps, FeedbackBoardSta
      defaultActionItemIteration: '',
      hasItems: false,
      isDataLoaded: false,
-      currentVoteCount: (props.board.boardVoteCollection === undefined || props.board.boardVoteCollection === null) ? "0" : (props.board.boardVoteCollection[this.props.userId] === undefined || props.board.boardVoteCollection[this.props.userId] === null) ? "0" : props.board.boardVoteCollection[this.props.userId]?.toString()
+      currentVoteCount: props.board.boardVoteCollection?.[this.props.userId]?.toString() ?? "0",


Thank you for trimming this down!

mindlessroman · 2023-05-02T20:23:40Z

RetrospectiveExtension.Frontend/components/__mocks__/mocked_components/mockedFeedbackColumn.tsx

@@ -90,7 +89,7 @@ testColumnsObj[testColumnUuidOne] = {
 testColumnsObj[testColumnUuidTwo] = {
  columnProperties:
  {
-    id: TooltipOverflowMode,


Appreciate the diligence! Always appreciated :D

mindlessroman · 2023-05-02T20:27:47Z

RetrospectiveExtension.Frontend/components/__tests__/feedbackBoard.test.tsx

+
+jest.mock('../feedbackBoardMetadataForm', () => mocked({}));
+jest.mock('azure-devops-extension-api/Work/WorkClient', () => {
+  const getTeamIterationsMock = () => {


If this chunk is a copy of the other, I think that's a valid thing to move to __mocks__ folder so there's less duplicate code. It's probably okay to remove any bit that's not explicitly called, but that may be worth doing a local test or two to be sure.

mindlessroman · 2023-05-02T20:38:17Z

RetrospectiveExtension.Frontend/components/__tests__/feedbackBoard.test.tsx

+}));
+
+jest.mock('azure-devops-extension-api/Common', () => ({
+  getClient: (clientClass: typeof Object) => new clientClass(),


I'll make a more overarching issue to maybe get the tests and components tidied eventually - may be a bit of an undertaking. For now, you should be good to go!

mindlessroman · 2023-05-02T20:40:37Z

RetrospectiveExtension.Frontend/components/__tests__/feedbackBoard.test.tsx

+  });
+
+  it(`shows correct vote counts`, () => {


Suggested change

it(`shows correct vote counts`, () => {

it(`shows correct vote counts when no votes have been cast`, () => {

optional: a test for when one or two different people have cast their votes? If not, this can have a TODO or something similar and can be tackled in a later code snippet.

mindlessroman · 2023-05-02T20:40:45Z

RetrospectiveExtension.Frontend/components/feedbackBoard.tsx

+      .flatMap((columnId) => this.state.columns[columnId].columnItems);
+    const participants = new Set(columnItems.flatMap((columnItem) => Object.keys(columnItem.feedbackItem.voteCollection))).size;
+
+    const teamVotes = columnItems


I love this solution! Great job!

kimsey0 added 2 commits December 9, 2022 10:43

Show total number of votes by all users and maximum number of votes f…

017d544

…or all users.

Add changelog entry.

e305dc5

kimsey0 added 6 commits February 2, 2023 16:51

Change exception handling azureDevOpsWorkService to not throw

4a5c24d

a new exception if the previous one doesn't have a serverError property

Extend mock ExtensionDataService with mock ExtensionDataManager

660432c

Change mock columns object to array to support map

d10fadd

Add simple test that the feedback board component can render

b74e7a4

Fix incorrect test column ID and revert unnecessary change from objec…

838dcd2

…t to array

Add first version of test for vote count display

0ecc80f

kimsey0 commented Feb 3, 2023

View reviewed changes

Move WorkItemTrackingRestClient class mock into setupTests

ddf9eea

mindlessroman approved these changes May 2, 2023

View reviewed changes

mindlessroman added feature: Vote Phase These issues are associated with the Vote and its associated functionality enhancement New feature or request labels May 2, 2023

mindlessroman linked an issue May 2, 2023 that may be closed by this pull request

Show total votes used by team together with my votes used #484

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show total number of votes and maximum number of votes for all users #488

Show total number of votes and maximum number of votes for all users #488

kimsey0 commented Dec 9, 2022

mindlessroman commented Dec 12, 2022

polatengin commented Dec 13, 2022

kimsey0 commented Feb 3, 2023

kimsey0 Feb 3, 2023

mindlessroman May 2, 2023

kimsey0 Feb 3, 2023

mindlessroman May 2, 2023

kimsey0 Feb 3, 2023

mindlessroman May 2, 2023

mindlessroman commented Feb 3, 2023

kimsey0 commented Apr 27, 2023

mindlessroman commented May 2, 2023

mindlessroman May 2, 2023

mindlessroman May 2, 2023

mindlessroman May 2, 2023

mindlessroman May 2, 2023

mindlessroman May 2, 2023

mindlessroman May 2, 2023

mindlessroman May 2, 2023

	it(`shows correct vote counts`, () => {
	it(`shows correct vote counts when no votes have been cast`, () => {

Show total number of votes and maximum number of votes for all users #488

Are you sure you want to change the base?

Show total number of votes and maximum number of votes for all users #488

Conversation

kimsey0 commented Dec 9, 2022

Pull Request

Description

Bug or Feature?

Fundamentals

Accessibility

Screenshots and Logs

mindlessroman commented Dec 12, 2022

polatengin commented Dec 13, 2022

kimsey0 commented Feb 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mindlessroman commented Feb 3, 2023

kimsey0 commented Apr 27, 2023

mindlessroman commented May 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment