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/work experience average section rating api #1383

Merged
merged 7 commits into from
Aug 18, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ const createLinkTo = ({ pageType, id }) => ({

const SNIPPET_SIZE = 30;

const averageSectionRating = 3.8; // for temporary use

const ExperienceEntry = ({
pageType,
data: {
Expand All @@ -34,6 +32,7 @@ const ExperienceEntry = ({
week_work_time: weekWorkTime,
salary,
recommend_to_others: recommendToOthers,
averageSectionRating,
},
size,
canView,
Expand Down Expand Up @@ -71,7 +70,7 @@ const ExperienceEntry = ({
)}
</div>
)}
{averageSectionRating !== null ? (
{averageSectionRating ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

不能這樣寫,否則這樣若出現0就會有問題

Copy link
Collaborator Author

@YongChenSu YongChenSu Jul 28, 2024

Choose a reason for hiding this comment

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

我有想過這個問題,但是不是不會有 0 的狀況出現啊?不過這樣寫,是很信任後端來的資料不為 0,好像也不是很好。

如果改成 averageSectionRating !== undefined,這樣寫由於 averageSectionRating 已經從 data 解構出來,averageSectionRating 有可能是 undefined,這樣改覺得好嗎?

喔,抱歉,發現我上面有講錯,ExperienceEntry 解構出來的 averageSectionRatingundefinednull,兩者都會有,我想一下怎麼改

Copy link
Contributor

Choose a reason for hiding this comment

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

image

這個 comment 當初是在討論 null / undefined,我覺得 averageSectionRating 會不存在才是重點

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

喔,抱歉,發現我上面有講錯,ExperienceEntry 解構出來的 averageSectionRatingundefinednull,兩者都會有,我想一下怎麼改

image

Copy link
Contributor

@mark86092 mark86092 Jul 28, 2024

Choose a reason for hiding this comment

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

咦,我看 code 應該只會有 undefined,竟然會有 null, undefined 都跑出來了

https://github.com/goodjoblife/WorkTimeSurvey-backend/compare/master...dev#diff-50883af379f9b860b4e4d913841f23c6008626ddfc4af43efbe3d6edf4d761a8R767

麻煩 @YongChenSu 可以分別給我

undefined 的 case
null 的 case

我覺得應該要再釐清一下

Copy link
Collaborator Author

@YongChenSu YongChenSu Jul 28, 2024

Choose a reason for hiding this comment

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

@mark86092 看起來是進到單篇 experiences 會出現 undefined
2024-07-28 22 48 46

Copy link
Contributor

@mark86092 mark86092 Jul 28, 2024

Choose a reason for hiding this comment

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

我們先從 api response 來看好了

(1) null case 的 experience id
(2) undefined case 的 experience id

麻煩 @YongChenSu 可以找這兩個 case 的給我嗎?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

我先釐清一下,或許可以自己解決

Copy link
Collaborator Author

@YongChenSu YongChenSu Aug 5, 2024

Choose a reason for hiding this comment

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

已修正,尚未推上去,發現是我沒將 averageSectionRating 加到 queryRelatedExperiencesGql 中。

趁機釐清一下,頁面資料來源

  1. 評價列表這一頁,是 sever side render,averageSectionRating 是從 work-experiences 來的
    Screen Shot 2024-08-05 at 8 43 53 PM
  2. 進到單篇,最上面的完整的評價,是 sever side render,averageSectionRating 是從 queryExperienceGql 來的
    image
  3. 進到單篇,下面的評價列表,averageSectionRating 是從 queryRelatedExperiencesGql 來的
    Screen Shot 2024-08-05 at 9 35 49 PM

<div className={styles.overallRatingWrapper}>
<OverallRating rating={averageSectionRating} />
</div>
Expand Down Expand Up @@ -112,6 +111,7 @@ const ExperienceEntry = ({
ExperienceEntry.propTypes = {
canView: PropTypes.bool.isRequired,
data: PropTypes.shape({
averageSectionRating: PropTypes.number,
created_at: PropTypes.string.isRequired,
id: PropTypes.string.isRequired,
job_title: PropTypes.shape({ name: PropTypes.string.isRequired })
Expand Down
4 changes: 2 additions & 2 deletions src/components/ExperienceDetail/Article/ArticleInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ InterviewInfoBlocks.propTypes = {

const WorkInfoBlocks = ({ experience, hideContent }) => {
const expInYearText = formatExperienceInYear(experience.experience_in_year);
const averageSectionRating = 3.62; // for temporary use
return (
<Fragment>
<InfoBlock
Expand Down Expand Up @@ -167,7 +166,7 @@ const WorkInfoBlocks = ({ experience, hideContent }) => {
</InfoBlock>
) : null}
<RatingInfo
rating={averageSectionRating}
rating={experience.averageSectionRating}
recommend={experience.recommend_to_others}
/>
</Fragment>
Expand All @@ -176,6 +175,7 @@ const WorkInfoBlocks = ({ experience, hideContent }) => {

WorkInfoBlocks.propTypes = {
experience: PropTypes.shape({
averageSectionRating: PropTypes.number,
company: PropTypes.shape({
name: PropTypes.string,
}),
Expand Down
2 changes: 1 addition & 1 deletion src/components/common/OverallRating/Rating.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const Rating = ({ rating, textYellow }) => (
[styles.textYellow]: !textYellow,
})}
>
{rating} 分
{rating.toFixed(1)} 分
</div>
);

Expand Down
18 changes: 8 additions & 10 deletions src/components/common/OverallRating/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@ import Thumbs from './Thumbs';
import { Rating, RatingLabel } from './Rating';
import styles from './Rating.module.css';

const OverallRating = ({ rating, hasRatingLabel }) => {
return (
<div className={cn(styles.overallRating)}>
<Rating rating={rating} textYellow={hasRatingLabel} />
<div className={styles.ratingInfo}>
<Thumbs rating={rating} />
{hasRatingLabel && <RatingLabel rating={rating} />}
</div>
const OverallRating = ({ rating, hasRatingLabel }) => (
<div className={cn(styles.overallRating)}>
<Rating rating={rating} textYellow={hasRatingLabel} />
<div className={styles.ratingInfo}>
<Thumbs rating={rating} />
{hasRatingLabel && <RatingLabel rating={rating} />}
</div>
);
};
</div>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

unexpected code change ?


OverallRating.propTypes = {
hasRatingLabel: PropTypes.bool,
Expand Down
1 change: 1 addition & 0 deletions src/graphql/company.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export const getCompanyQuery = /* GraphQL */ `
reply_count
like_count
recommend_to_others
averageSectionRating
}
salary_work_times {
id
Expand Down
2 changes: 2 additions & 0 deletions src/graphql/experience.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export const queryExperienceGql = /* GraphQL */ `
}
week_work_time
recommend_to_others
averageSectionRating
}
}
}
Expand Down Expand Up @@ -151,6 +152,7 @@ export const queryRelatedExperiencesGql = /* GraphQL */ `
}
week_work_time
recommend_to_others
averageSectionRating
}
}
}
Expand Down
Loading