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

total-positions-addition-public-config-api-UI #172

Merged
merged 8 commits into from
Oct 1, 2024

Conversation

karthik8239
Copy link
Contributor

##total-postions-addition-public-config-api-UI

##API update and Ui addition

Changes performed

  • Updated the seedcache.ts to include total positions in "general memory storage" from "job postings" table
  • Update the '/api/public/config' to include total positions in the api response fetching from general storage
  • Updated the setup-state.ts file to include the total positions in the state value "totalPositionsState"
  • Included the value in the UI index.vue beside "open positions"

[Attach screenshots or videos demonstrating the new feature in action.]
image

Issue Link

#126

Acceptance Criteria

The number of open positions is displayed next to the "Open Positions" header.
Total positions should be taken from cache (general_memoryStorage).
The counter updates automatically when job positions are added, edited, or removed (integrate update in endpoint).
The display should follow the format: "Open Positions (X)", where X is the number of positions.

app/pages/index.vue Outdated Show resolved Hide resolved
app/pages/index.vue Outdated Show resolved Hide resolved
@karthik8239
Copy link
Contributor Author

karthik8239 commented Sep 24, 2024

fixed comments and the
pull request ready for review @amandesai01 @shivam-sharma7

<h3 class="text-lg leading-snug text-zinc-600 font-bold mt-8 mb-2">Open Positions</h3>
<h3 class="text-lg leading-snug text-zinc-600 font-bold mt-8 mb-2">
Open Positions
<span class="text-sm text-zinc-500 before:content-['('] after:content-[')']">>{{ totalPositions }}</span>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<span class="text-sm text-zinc-500 before:content-['('] after:content-[')']">>{{ totalPositions }}</span>
<span class="text-sm text-zinc-500 before:content-['('] after:content-[')']">{{ totalPositions }}</span>

why extra >?

Copy link
Member

@shivam-sharma7 shivam-sharma7 Sep 25, 2024

Choose a reason for hiding this comment

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

Also, See the following img. I want something like that and replace Applicants with Positions that's what I was saying here-> #172 (comment)

image

@shivam-sharma7
Copy link
Member

@karthik8239 Above suggestion need to be completed and please, always create new branch for contributing, for now we will handle.

@karthik8239
Copy link
Contributor Author

@karthik8239 Above suggestion need to be completed and please, always create new branch for contributing, for now we will handle.

Hello shivam,sure I will create a new branch from now I fixed the comment and pushed latest changes.Please review changes and approve my PR.

@@ -42,8 +42,8 @@ useSeoMeta({
<main class="grow w-full lg:w-2/3 mx-auto mt-20 p-2">
<SiteHeader />
<h3 class="text-lg leading-snug text-zinc-600 font-bold mt-8 mb-2">
Open Positions
<span class="text-sm text-zinc-500 before:content-['('] after:content-[')']">>{{ totalPositions }}</span>
Open Applicants
Copy link
Member

Choose a reason for hiding this comment

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

Why this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To add "()" around displaying the number through css instead of text as per @shivam-sharma7 comment.

Copy link
Member

Choose a reason for hiding this comment

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

I asked for Open Applicants. Why that? we want open postings only.

Copy link
Member

Choose a reason for hiding this comment

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

Shivam was showing the format in which you have to display number of postings.

Comment on lines 23 to 28
const activePostings = await db
.select({ totalApplicants: jobPostingsTable.totalApplicants })
.from(jobPostingsTable)
.where(eq(jobPostingsTable.isPublished, true));

const totalActivePostings = activePostings.length;
Copy link
Member

Choose a reason for hiding this comment

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

you need to run count query to get count.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the goal is to get total applicants ? not active postings?

Copy link
Member

Choose a reason for hiding this comment

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

you have to show posting count, not applicants count. Reason we need that from database is because we have future plans in cache management and paging side, so frontend won't be able to show length directly.

@amandesai01 amandesai01 merged commit e4d1512 into profilecity:main Oct 1, 2024
1 check passed
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.

3 participants