Modify company data schema to include licensing details #1146

jfkonecn · 2024-08-27T18:31:11Z

closes oncokb/oncokb-pipeline#442

zhx828

I will circle back to review the frontend once the backend changes are finalized.

zhx828 · 2024-08-29T14:57:25Z

src/main/resources/config/liquibase/changelog/20210927165433_added_entity_Company.xml

@@ -40,6 +40,9 @@
            <column name="legal_contact" type="varchar(255)">
                <constraints nullable="true" />
            </column>
+            <column name="additional_info" type="${clobType}">


To work with the production database, we need to create a new changeSet instead of adding to the old one.

zhx828 · 2024-08-29T14:57:42Z

src/main/resources/config/liquibase/changelog/20210927165433_added_entity_Company.xml

            <!-- jhipster-needle-liquibase-add-loadcolumn - JHipster (and/or extensions) can add load columns here -->
        </loadData>
    </changeSet>
-    <changeSet id="20210927165433-2-data" author="calvin" context="faker">


This should not be deleted I believe

zhx828 · 2024-08-29T15:00:21Z

src/main/java/org/mskcc/cbio/oncokb/service/dto/CompanyDTO.java

@@ -41,9 +41,11 @@ public class CompanyDTO implements Serializable {

    @NotEmpty
    private Set<String> companyDomains = new HashSet<>();
+    @Lob
+    private String additionalInfo;


We should have an AdditionalInfo class to host all properties, similar to AdditionalInfoDTO under useradditionalinfo

zhx828 · 2024-08-29T15:00:59Z

src/main/resources/config/liquibase/fake-data/blob/hipster.txt

@@ -0,0 +1 @@
+{}


Let's don't include hipster.png and hipster.txt under the fake-data folder.

zhx828 · 2024-09-05T15:00:07Z

screenshot-test/data/api-companies.json

+    "additionalInfo": {
+      "license": {
+        "termination": {
+          "date": "2024-09-21",


I know this is a fake data, but can we have termination date after activation date? 😃

zhx828 · 2024-09-05T15:05:47Z

src/main/java/org/mskcc/cbio/oncokb/service/dto/companyadditionalinfo/CompanyLicense.java

+import java.time.LocalDate;
+
+public class CompanyLicense implements Serializable {
+  private LocalDate activation;


We have been suing Instant for dates, let's do that.

I'm using LocalDate since it represents a date only. I'm assuming we don't want to deal with time zones?

We've been using instant with timezone and map it to local in the frontend.

zhx828 · 2024-09-05T15:05:54Z

src/main/java/org/mskcc/cbio/oncokb/service/dto/companyadditionalinfo/CompanyTermination.java

+
+public class CompanyTermination implements Serializable {
+  private Integer notificationDays;
+  private LocalDate date;


zhx828 · 2024-09-05T15:14:08Z

...main/resources/config/liquibase/changelog/20240829165533_added_additionalInfo_to_Company.xml

+      xmlns:ext="http://www.liquibase.org/xml/ns/dbchangelog-ext"
+      xmlns:pro="http://www.liquibase.org/xml/ns/pro"


IntelliJ indicates these two are not used.

I copy and pasted from liquibase's doc. I guess they need to update them. 😆

zhx828 · 2024-09-05T15:14:13Z

...main/resources/config/liquibase/changelog/20240829165533_added_additionalInfo_to_Company.xml

+          http://www.liquibase.org/xml/ns/dbchangelog-ext http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd
+          http://www.liquibase.org/xml/ns/pro http://www.liquibase.org/xml/ns/pro/liquibase-pro-latest.xsd">


IntelliJ indicates these two are not used.

zhx828 · 2024-09-05T15:15:13Z

...main/resources/config/liquibase/changelog/20240829165533_added_additionalInfo_to_Company.xml

+          http://www.liquibase.org/xml/ns/dbchangelog-ext http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd
+          http://www.liquibase.org/xml/ns/pro http://www.liquibase.org/xml/ns/pro/liquibase-pro-latest.xsd">
+
+  <changeSet author="john" id="20240829165533">


We seem to use jhipster in the author field in the past which is wrong. Let's use oncokb going forward.

so author="oncokb"?

zhx828 · 2024-09-05T15:22:19Z

src/main/webapp/app/pages/companyPage/CompanyAdditionalInfo.tsx

+import { CompanyAdditionalInfoDTO } from 'app/shared/api/generated/API';
+import { FormTextAreaField } from 'app/shared/textarea/FormTextAreaField';
+
+const defaultAdditionalInfo: CompanyAdditionalInfoDTO = {


Having a default means we will always have companyAdditionalInfo populated in the DB correct? Historically we don't add the default but leave the filed empty in the DB.

Yes, but shouldn't we always have it populated going forward?

When you say empty do you mean that you don't include the empty field at all in the json?

Yeah, if the user does not have any additional info, the DB has no content.

zhx828 · 2024-09-05T15:37:01Z

src/main/webapp/app/pages/companyPage/CompanyAdditionalInfo.tsx

+          <FormInputField
+            id="termination.date"
+            label="Termination Date"
+            type="date"


The newly added components seem to have different height than others?

I was trying to copy FormTextAreaField is that a bad example?

Not sure. If this component comes from us, we need to examine why it looks differently from others.

zhx828 · 2024-09-05T15:37:48Z

src/main/webapp/app/shared/select/FormSelectWithLabelField.tsx

@@ -1,23 +1,25 @@
 import React from 'react';
 import Select from 'react-select';

-export type IFormSelectWithLabelProps = {
-  onSelection: (selectedOption: any) => void;
+export type IFormSelectWithLabelProps<Label, Value> = {


jfkonecn · 2024-09-09T20:49:47Z

src/main/webapp/app/components/newCompanyForm/NewCompanyForm.tsx

+  @observable companyAdditionalInfo: CompanyAdditionalInfoDTO = {
+    license: {
+      ...defaultInfo.license,
+      activation: new Date().toISOString().split('T')[0],


only if creating a regular company

jfkonecn · 2024-09-09T20:53:22Z

src/main/webapp/app/components/newCompanyForm/NewCompanyForm.tsx

+    activation: '',
+    termination: {
+      date: '',
+      notificationDays: 60,


auto-renew true

add warning to user that email will be sent if inside of notification window when the user updates the company page.

zhx828

after selecting activation, the auto-renewal should be Yes
Termination Notification Days and notes should not be editable if termination date is not available
termination date should not be earlier than activation
add warning to user that email will be sent if inside of notification window when the user updates the company page. this is not implemented. See the modal when converting license status from regular to trial as an example

This reverts commit 8d1cdd8.

jfkonecn · 2024-09-26T17:36:18Z

after selecting activation, the auto-renewal should be Yes

Termination Notification Days and notes should not be editable if termination date is not available

termination date should not be earlier than activation

add warning to user that email will be sent if inside of notification window when the user updates the company page. this is not implemented. See the modal when converting license status from regular to trial as an example

@zhx828 I addressed everything but "add warning to user that email will be sent if inside of notification window when the user updates the company page. this is not implemented. See the modal when converting license status from regular to trial as an example"

zhx828

Thanks! LGTM.

* Modify company data schema to include licensing details (#1146) * Added termination warning email button (#1151) * Added endpoints to send company notification email (#1152) * Bump elliptic from 6.5.2 to 6.5.7 Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.2 to 6.5.7. - [Commits](indutny/elliptic@v6.5.2...v6.5.7) --- updated-dependencies: - dependency-name: elliptic dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> * allow keypress events for textareas * Fix data download (#1162) * User is granted API access on approval (#1163) * Added endpoints to send notification email * Data version is now displayed (#1165) * Addressed PR comments * Updated Setup Java in Git Actions (#1166) --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Hongxin <[email protected]> Co-authored-by: bprize15 <[email protected]> Co-authored-by: bprize15 <[email protected]> Co-authored-by: Calvin Lu <[email protected]> --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Hongxin <[email protected]> Co-authored-by: bprize15 <[email protected]> Co-authored-by: bprize15 <[email protected]> Co-authored-by: Calvin Lu <[email protected]>

jfkonecn requested a review from zhx828 August 27, 2024 18:31

jfkonecn marked this pull request as draft August 28, 2024 12:46

Modify company data schema to include licensing details

db4386e

jfkonecn force-pushed the feature/company-data branch from e58c200 to db4386e Compare August 28, 2024 16:26

jfkonecn marked this pull request as ready for review August 28, 2024 16:34

zhx828 requested changes Aug 29, 2024

View reviewed changes

Now using object for additional info

88a4dd4

jfkonecn requested a review from zhx828 September 3, 2024 15:43

fixed tests

ebc63f7

zhx828 requested changes Sep 5, 2024

View reviewed changes

jfkonecn commented Sep 9, 2024

View reviewed changes

jfkonecn added 3 commits September 10, 2024 15:57

Addressed PR comments

f9a2255

Fixed screenshot tests

7f642e4

Fixed screenshot tests

170e000

jfkonecn requested a review from zhx828 September 10, 2024 20:35

jfkonecn added the feature Feature tag for release label Sep 10, 2024

zhx828 requested changes Sep 16, 2024

View reviewed changes

jfkonecn added 5 commits September 26, 2024 08:10

fixed merge conflicts

3f18590

Addressed PR comments

d0e800d

Added a warning if within email notification window

8d1cdd8

Revert "Added a warning if within email notification window"

8ae8022

This reverts commit 8d1cdd8.

Addressed PR comments

37c2dd7

jfkonecn requested a review from zhx828 September 26, 2024 17:35

zhx828 changed the base branch from master to feature/company-license October 1, 2024 19:48

zhx828 approved these changes Oct 1, 2024

View reviewed changes

jfkonecn merged commit b47aefe into oncokb:feature/company-license Oct 1, 2024
6 checks passed

jfkonecn deleted the feature/company-data branch October 1, 2024 20:29

jfkonecn mentioned this pull request Oct 8, 2024

Added termination warning email button #1151

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify company data schema to include licensing details #1146

Modify company data schema to include licensing details #1146

jfkonecn commented Aug 27, 2024

zhx828 left a comment

zhx828 Aug 29, 2024

zhx828 Aug 29, 2024

zhx828 Aug 29, 2024

zhx828 Aug 29, 2024

zhx828 Sep 5, 2024

zhx828 Sep 5, 2024

jfkonecn Sep 5, 2024

zhx828 Sep 9, 2024

zhx828 Sep 5, 2024

zhx828 Sep 5, 2024

jfkonecn Sep 5, 2024

zhx828 Sep 5, 2024

zhx828 Sep 5, 2024

jfkonecn Sep 5, 2024

zhx828 Sep 9, 2024

zhx828 Sep 5, 2024

jfkonecn Sep 5, 2024 •

edited

Loading

zhx828 Sep 9, 2024

zhx828 Sep 5, 2024

jfkonecn Sep 5, 2024

zhx828 Sep 9, 2024

zhx828 Sep 5, 2024

jfkonecn Sep 9, 2024 •

edited

Loading

jfkonecn Sep 9, 2024

jfkonecn Sep 9, 2024

jfkonecn Sep 9, 2024

zhx828 left a comment

jfkonecn commented Sep 26, 2024

zhx828 left a comment

		xmlns:ext="http://www.liquibase.org/xml/ns/dbchangelog-ext"
		xmlns:pro="http://www.liquibase.org/xml/ns/pro"

		http://www.liquibase.org/xml/ns/dbchangelog-ext http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd
		http://www.liquibase.org/xml/ns/pro http://www.liquibase.org/xml/ns/pro/liquibase-pro-latest.xsd">

Modify company data schema to include licensing details #1146

Modify company data schema to include licensing details #1146

Conversation

jfkonecn commented Aug 27, 2024

zhx828 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfkonecn Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfkonecn Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhx828 left a comment

Choose a reason for hiding this comment

jfkonecn commented Sep 26, 2024

zhx828 left a comment

Choose a reason for hiding this comment

jfkonecn Sep 5, 2024 •

edited

Loading

jfkonecn Sep 9, 2024 •

edited

Loading