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: user recent openings new #431

Closed
wants to merge 49 commits into from
Closed

Conversation

jazzgrewal
Copy link
Collaborator

@jazzgrewal jazzgrewal commented Oct 29, 2024

Description

PR Description
Feature: Recent Openings History for the nr-silva Application

This pull request introduces a new feature that implements a browser history-like functionality for openings in the nr-silva application.

Changes Made:

User Interaction Tracking:

Whenever a user searches for openings and selects one to view, a request is sent to the backend. This interaction is logged in the user_recent_openings table within the User Preference database (PostgreSQL).
Recent Openings Dashboard:

Users can now see a "Recent Openings" table on the Openings Dashboard. This table displays the openings that the user has recently viewed, sorted by lastViewTime.
Details for each opening are fetched from the Oracle database, providing users with comprehensive information about their recent activities.
Database Modifications:

Added the necessary tables and fields in the PostgreSQL database to support the new functionality.
Frontend and Backend Integration:

This feature involves updates to both the frontend and backend components, ensuring seamless interaction and data flow between the two. By implementing this feature, we enhance user experience by allowing users to easily track and revisit their recently viewed openings.

Fixes # (issue)
This feature addresses the following tickets:

SILVA-524: https://apps.nrs.gov.bc.ca/int/jira/browse/SILVA-524
SILVA-534: https://apps.nrs.gov.bc.ca/int/jira/browse/SILVA-534
SILVA-537: https://apps.nrs.gov.bc.ca/int/jira/browse/SILVA-537

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Tested Locally Docker Compose
  • Unit Tests for React components

Checklist

  • I have read the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have already been accepted and merged

Further comments


Thanks for the PR!

Any successful deployments (not always required) will be available below.

Backend: https://nr-silva-431-backend.apps.silver.devops.gov.bc.ca/actuator/health
Frontend: https://nr-silva-31-frontend.apps.silver.devops.gov.bc.ca

Once merged, code will be promoted and handed off to following workflow run.
Main Merge Workflow


Thanks for the PR!

Any successful deployments (not always required) will be available below.

Backend: https://nr-silva-431-backend.apps.silver.devops.gov.bc.ca/actuator/health
Frontend: https://nr-silva-31-frontend.apps.silver.devops.gov.bc.ca

Once merged, code will be promoted and handed off to following workflow run.
Main Merge Workflow

Copy link
Member

@DerekRoberts DerekRoberts left a comment

Choose a reason for hiding this comment

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

What does this PR achieve? Is there an issue number, since this one hasn't been provided yet?

If the PR isn't ready for review please change status to draft.

@@ -0,0 +1,284 @@
import { ITableHeader } from "../../../../types/TableHeader";
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this file for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is for the headers for the table that is rendered, we dont have a unique on for each table yet. probably a good idea to create one for each.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are some hardcoded data down below. Where it is used? Is this some old, hardcoded data? If it is, please remove it. If is not, please reacess the necessity of keeping this specific data array on this file.

If this file is used to ser the data table header only, please rename it to something more meaningful and remove this hardcoded data, or move the hardcoded data to the test folder to be used by tests.

frontend/src/screens/Reports/Reports.scss Outdated Show resolved Hide resolved
Copy link
Contributor

@paulushcgcj paulushcgcj left a comment

Choose a reason for hiding this comment

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

There a few things that can be improved yet. I will check out the code later to test a few other things as well

Copy link
Contributor

@paulushcgcj paulushcgcj left a comment

Choose a reason for hiding this comment

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

I think this is the last review. Apart from a few files that require some attention, it seems like a good PR. I did some small code changes and already updated everything I touched.

Please address those small points and we should be good to go

@@ -0,0 +1,284 @@
import { ITableHeader } from "../../../../types/TableHeader";
Copy link
Contributor

Choose a reason for hiding this comment

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

There are some hardcoded data down below. Where it is used? Is this some old, hardcoded data? If it is, please remove it. If is not, please reacess the necessity of keeping this specific data array on this file.

If this file is used to ser the data table header only, please rename it to something more meaningful and remove this hardcoded data, or move the hardcoded data to the test folder to be used by tests.

Copy link

sonarqubecloud bot commented Nov 4, 2024

Quality Gate Failed Quality Gate failed for 'nr-silva-frontend'

Failed conditions
64.5% Coverage on New Code (required ≥ 80%)
42.8% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

Copy link

sonarqubecloud bot commented Nov 4, 2024

jazzgrewal and others added 22 commits November 5, 2024 11:16
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…le Multi-Select Dropdown (#442)

Co-authored-by: Paulo Gomes da Cruz Junior <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@DerekRoberts
Copy link
Member

@jazzgrewal This PR has a number of files marked as changed that probably haven't been. Would you like a hand cleaning up the PR's history? @paulushcgcj

@jazzgrewal
Copy link
Collaborator Author

@jazzgrewal This PR has a number of files marked as changed that probably haven't been. Would you like a hand cleaning up the PR's history? @paulushcgcj

Hi @DerekRoberts , yes this has become a big mess. I am considering to open separate individual branches and redoing all the changes by copying over the new stuff from here. Its just too annoying now.

@jazzgrewal jazzgrewal closed this Nov 7, 2024
@DerekRoberts DerekRoberts deleted the feat/userRecentOpeningsNew branch November 13, 2024 02:49
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