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

Feature: Allow Custom Property Update in Glossary Bulk Import/export #17919

Merged
merged 66 commits into from
Sep 30, 2024

Conversation

sonika-shah
Copy link
Contributor

@sonika-shah sonika-shah commented Sep 19, 2024

Describe your changes:

  • User should be able update custom property when performing a bulk upload at glossary level
Screen.Recording.2024-09-30.at.1.02.12.AM.mov
Screen.Recording.2024-09-30.at.6.04.58.PM.mov

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

* @param field The input string with key-value pairs.
* @return A list of key-value pairs, handling quotes and semicolons correctly.
*/
public static List<String> fieldToExtensionStrings(String field) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we remove these comments.
can we add some mocks to test this part of code

Copy link
Contributor Author

@sonika-shah sonika-shah Sep 25, 2024

Choose a reason for hiding this comment

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

updated logic of fieldToExtensionStrings to use CSVParser with added tests

@harshach
Copy link
Collaborator

@sonika-shah fieldToInternalArray is unnecessarily complex. You can write the same thing like below using CSVParser

import org.apache.commons.csv.CSVFormat;
import org.apache.commons.csv.CSVParser;
import org.apache.commons.csv.CSVRecord;

public static List<String> fieldToExtensionStrings(String field) throws IOException {
    if (field == null || field.isBlank()) {
        return List.of();
    }

    CSVFormat format = CSVFormat.DEFAULT
            .withDelimiter(';')
            .withQuote('"')
            .withEscape('"') 
            .withRecordSeparator(null); 

    try (CSVParser parser = CSVParser.parse(field, format)) {
        List<String> result = new ArrayList<>();
        for (CSVRecord record : parser) {
            for (String value : record) {
                result.add(value);
            }
        }
        return result;
    }
}```

Copy link

Copy link

@sonika-shah sonika-shah changed the title Feature: Allow Custom Property Update in Bulk Upload Feature: Allow Custom Property Update in Glossary Bulk Import/export Sep 30, 2024
@Ashish8689 Ashish8689 merged commit 1d727d5 into main Sep 30, 2024
24 of 27 checks passed
@Ashish8689 Ashish8689 deleted the feature-import-export-custom-property branch September 30, 2024 18:42
Ashish8689 added a commit that referenced this pull request Sep 30, 2024
…17919)

* fix import issue

* Feat : Allow Custom Property Update in Bulk Upload

* Feat : Allow Custom Property Update in Bulk Upload

* supported editable imports in glossary page

* added remaning localizaion keys

* update logic of fieldToExtensionStrings to use csvparser

* update json and partialStatus condition

* fix tests for partialSuccess status change

* supported customProperty editable field

* fix error in custom property edit modal on new line empty custom property

* added entity type from root to support other bulk import entity as well

* fix the quote removing due to the regex in the string type

* Add backend tests , and error msg improvements

* GlossaryStatus header change

* fix unit test and dry run in case of synonyms having quotes in it

* Remove extension column in CSVs for all entities except glossaryTerm

* added editor for reviewers

* unit test around csv utils

* added escape for string too, in case of semicolon comes

* added playwright test without extension and supported relatedTerm as editable

* added unit test around csv util logic

* resolve conflicts

* Backend - add support for enumWithDescriptions in bulk import

* add tests and other error handling improvements related to enumWithDescriptions

* fix the custom property modal header and render the layout as per right panel in entities

* parese enumWithDescription for the customProperty modal while editable

* fix description data in enumWithDescription one

* fix: Handle NullPointerException when adding custom properties to ensure loop continues for other schemas of the same type for addToRegistry

* added extension playwrigth test and fix enumWithDescription object failure

* descrease the size of extension  modal

* remove additional comments

* fix the escape in parent key

* improve custom property layout

* improve ui for inline properties

* fix description, glossary and relatedTerm escape char issue

* fix some customProperty ui changes

* fix sonar issue

* minor layout changes

* minor label improvements for entity ref and list

---------

Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com>
Co-authored-by: Ashish Gupta <ashish@getcollate.io>
Co-authored-by: mohitdeuex <mohit.y@deuexsolutions.com>
Co-authored-by: Sachin Chaurasiya <sachinchaurasiyachotey87@gmail.com>
(cherry picked from commit 1d727d5)
Sachin-chaurasiya added a commit that referenced this pull request Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend safe to test Add this label to run secure Github workflows on PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants