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

Convert pik to cik #1109

Merged
merged 11 commits into from
Feb 19, 2024
Merged

Conversation

qnhn22
Copy link
Contributor

@qnhn22 qnhn22 commented Jan 7, 2024

Description

I have created a new state called 'Cik' for client-side usage. The code has been refactored to eliminate the need for the 'pik' array, replacing it with the newly introduced 'Cik' state. In order to make 'Cik' accessible, I have developed a new API that fetches 'Cik' data, which includes updating the model, creating an SQL file for retrieving information from the database, and implementing a new route.

Additionally, since the 'pik' is no longer required, the unit index can be removed from the unit table and refactor the code to eliminate any usage of the unit index. (database changed)

Fixes #901

(In general, OED likes to have at least one issue associated with each pull request. Replace [issue] with the OED GitHub issue number. In the preview you will see an issue description if you hover over that number. You can create one yourself before doing this pull request. This is where details are normally given on what is being addressed. Note you should not use the word "Fixes" if it does not completely address the issue since the issue would automatically be closed on merging the pull request. In that case use "Partly Addresses #[issue].)

Type of change

(Check the ones that apply by placing an "x" instead of the space in the [ ] so it becomes [x])

  • Note merging this changes the database configuration.
  • This change requires a documentation update

Checklist

(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)

  • I have followed the OED pull request ideas
  • I have removed text in ( ) from the issue request
  • You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section.

Limitations

(Describe any issues that remain or work that should still be done.)

Copy link
Member

@huss huss left a comment

Choose a reason for hiding this comment

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

Thanks to @qnhn22 for working on this issue. Testing indicates it works as expected. I have a few modest comments to address. Also, I noticed that a search reveals places where "pik" and "conversionArray" still appear (or are part of a word) in the code or comments. I looked at several of them and it is not clear to me that they are needed. Do you want to check them and remove as possible or is that something I need to do?

@huss
Copy link
Member

huss commented Feb 19, 2024

I just pushed changes to this PR. They do the final removal for the unit index from code and DB. It also merged in development to be sure there are no issues. I noted in the description that this changes the DB configuration and announced on Discord. With @qnhn22 changes and these, this PR is hopefully ready to merge. This completes the design doc steps until "Integrating cik".

@huss huss merged commit 6fec6a2 into OpenEnergyDashboard:development Feb 19, 2024
3 checks 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.

convert from pik array to cik state
2 participants