[component] Party Card

[Th1nkK1D]

• Stateless component, all data can be provided with props
• Since the link to explore page will also apply filter, filters must be able to pass by query parameters

Depends on

• 21

Used by

• 5

[nathakits]

What does the party chart data structure look like? Also the ดูคำสัญญา number is computed from the chart correct?

[nathakits]

Also the chart would be its own component right? Since it's used throughout the site.

[Th1nkK1D]

What does the party chart data structure look like? Also the ดูคำสัญญา number is computed from the chart correct?

In my opinion, the data should be a pair of PromiseStatus (defined in model/promises.ts) and count for example

[
  {status: PromiseStatus.Nodata, count: 20},
  {status: PromiseStatus.Pending, count: 10},
]

Being an array makes it easy to iterate over to make a stacked bar chart (It can be just divs in flex-box). But using object key-value like { [PromiseStatus.Pending]: 20 } or split it into separate props like <PartyCard :nodata="20" :pending="20" :> is also possible. You can spec the props which IndexPage will then aggregate and reshape the data before passing into this component. Then the total number of promises can be calculated from the same aggregated data as well (Sum it all up).

Also the chart would be its own component right? Since it's used throughout the site.

Just the PromiseOverview will have a similar chart. But that chart also has an icon and label. So I think we don't need to share the same component. Doing it separately would be simpler.

Please let me know what do you think 😄

[nathakits]

Sounds good, I will try your approach.

Also another question, I'm trying to call this.partyPromisesSum in another computed function but typescript throws this error below. Any idea how to resolve this error?

The promisesPercentage computed function runs fine, just the typescript that is throwing this error.

(work in progress)

computed: {
    partyPromisesSum(): string {
      const promiseStatus = this.partyPromises as PromiseStatus[];
      let sum = '';
      promiseStatus.forEach((el) => {
        sum = Object.values(el).reduce((a, b) => a + b);
      });
      return sum;
    },
    promisesPercentage() {
      const promiseStatus = this.partyPromises as PromiseStatus[];
      const promises = Object.values(promiseStatus[0]);
      const percentage = promises.map(
        (num) => (Number(num) / Number(this.partyPromisesSum)) * 100 // this.partyPromisesSum throws a typescript error
      );
      return percentage;
    },
}

TS2339: Property 'partyPromisesSum' does not exist on type 'CombinedVueInstance<Vue, unknown, unknown, unknown, Readonly<{ partyLogo: string; partyName: string; partyPromises: PromiseStatus[]; buttonUrl: string; }>>'.

UPDATE I think I found the solution (this as any).partyPromisesSum seems to resolve the typescript error

[nathakits]

Can someone help me review this component and give me feedback? I think there are more tests that can be added for this component. I've never done testing so I'm not sure what else to test.

[Th1nkK1D]

I'm regretting using Vue2 with TypeScript now :( Feel like the ecosystem causing us a lot of trouble. I can have a look on Monday. cc. @mixth might want to have a look at his test.

[mixth]

Let's explore all possible renderings for this component. We have 4 props:

• partyLogo -> have not been tested ❌
• partyName -> tested ⭕️
• partyPromises -> partially tested 🔸
• buttonUrl -> tested ⭕️

partyLogo

The question to test this prop is, what will happen when we:

1. have input the path -> the component shows the party logo (from image configuration)
2. have not -> the component shows dummy party logo (from image configuration)

Which give us possible test cases as:

1. It should load party logo from correct config path
   • Arrange: Fix the config path to known value
   • Act: Mount component with party logo name
   • Assert: The logo is sourced from the fixed config path with the party logo name
2. It should load dummy party logo when party logo is not given
   • Arrange: Fix the config path to known value
   • Act: Mount component WITHOUT party logo name
   • Assert: The logo is a dummy logo sourced from the fixed config path

hint: might be able to mock $config like in jest-setup.ts

partyPromises

This is an interesting prop to be tested. There are two affected areas when changing this props:

The total count

There is one path that has not been tested yet (highlighted in red below).

To test this, you may try having multiple items in partyPromises, preferably different counts, and check for total count on dom.

The bar chart

You can try having multiple items in partyPromises and then, check for the width-[*] attribute in dom if the calculation is correct.

Since test cases shall not be the repetition of the logic that is under test, it's better to have a fixed known value to pass to component and to inspect the component.

Example

For Input:

[ status: PromiseSatus.NoData, count: 1 }, { status: PromiseStatus.Proposed, count: 3 }]

We should expected:

get('.bg-color-[#8f8f8f]').classes().includes('w-[25%]')
get('.bg-color-[#fd9154]').classes().includes('w-[75%]')

Extra

When running yarn test, it also produces a code/test coverage report. Which can give you a quick overview of how your test cases "perform" over your code.

In this case, you can see what line test cases have never visited and create new test cases from there.

Note that, test coverage is not everything, it's just one metric worth looking. Having 100% coverage does not mean we do a good job in testing.

---

Writing this in English is harder than I thought. 😬 If you have any question or not sure how to implement those above, you can hit me up on Slack! Keep up your good work krub 😃

[Th1nkK1D]

For Input:

[ status: PromiseSatus.NoData, count: 1 }, { status: PromiseStatus.Proposed, count: 3 }]

We should expected:

get('.bg-color-[#8f8f8f]').classes().includes('w-[25%]')
get('.bg-color-[#fd9154]').classes().includes('w-[75%]')

Tailwind purge unused class by looking at the source code. So if w-[*] is dynamically generated, it's not working when we build. In this case, we should use the CSS height style instead. And I think @nathakits is doing this way. For the bg color, I recommended using the Tailwind classes so we don't need to declare the hex code again.

We can use Thai na krub hahaha Don't want to make the communication inconvenient. 😅

The rest is very informative. Thank you both!

[nathakits]

@mixth Wow, thank you for the very detailed explanation kub. Very informative.

@Th1nkK1D Ok kub. I have now updated the bg color to use Tailwind classes.

[nathakits]

@mixth I added all the tests as outlined. I think the component is now ready kub. Please take a look when you can kub.

EDIT: Sorry, the component is not ready.

[Th1nkK1D]

Do you need any help?

[nathakits]

@Th1nkK1D Sorry kub, I was very busy last week. I will try and finish this component within this week kub.

[Th1nkK1D]

@nathakits Don't worry krub, I didn't mean to put pressure. Just worrying that you might stuck at something cause we introduce you a lot of new things. So if you are stuck just reach out to us. Take your time krub phom!

[nathakits]

Ok kub. If I'm stuck, I will reach out.

[nathakits]

For the ดูคำสัญญา button, what link should the button be? Does it have some kind of params for filtering?

[Th1nkK1D]

For the ดูคำสัญญา button, what link should the button be? Does it have some kind of params for filtering?

Yes, you can send a query param to the explore page like /explore?party=ก้าวไกล