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

Extend VocabularlyManager to include ui-concerto label creation logic #374

Closed
mttrbrts opened this issue Jan 13, 2022 · 6 comments · Fixed by #386
Closed

Extend VocabularlyManager to include ui-concerto label creation logic #374

mttrbrts opened this issue Jan 13, 2022 · 6 comments · Fixed by #386
Assignees
Labels
Difficulty: Starter Good First Issue :octocat: Good for newcomers Type: Enhancement ✨ Improvement to process or efficiency Type: Feature Request 🛍️ New feature or request

Comments

@mttrbrts
Copy link
Member

Feature Request 🛍️

The new VocabularlyManager introduced in #361 includes a missing term generator, https://github.com/accordproject/concerto/blob/master/packages/concerto-vocabulary/lib/vocabularymanager.js#L59-L62.

Similarly the ui-concerto package in web-components includes some logic to automatically generate terms from model field names. https://github.com/accordproject/web-components/blob/master/packages/ui-concerto/src/utilities.js#L20-L25

These two features should be combined in this package, and ui-concerto refactored to use the common implementation.

@mttrbrts mttrbrts added Difficulty: Starter Good First Issue :octocat: Good for newcomers Type: Enhancement ✨ Improvement to process or efficiency Type: Feature Request 🛍️ New feature or request labels Jan 13, 2022
@Jeevan-Kiran-Lenka
Copy link

Hey, @mttrbrts I would like to work on this issue since I'm very overwhelmed don't know from where to start. Can you please assign this to me as I have to get started :)

@mttrbrts
Copy link
Member Author

Welcome @Jeevan-Kiran-Lenka!

In summary, you will need to make changes in two places:

  1. Move the logic from ui-concerto to concerto-vocabulary (line links above).
  2. Refactor ui-concerto to import concerto-vocabulary and then call the missing term generator function.

@Jeevan-Kiran-Lenka
Copy link

Jeevan-Kiran-Lenka commented Jan 18, 2022

Thanks for the tip @mttrbrts !! Now it is easy to fix this :)

@Jeevan-Kiran-Lenka
Copy link

Hey!!, @mttrbrts I had some of the following doubts while working on this issue, I would really appreciate it if you can help me solve that.

  1.  export const normalizeLabel = (labelName = '') => labelName
     .replace(/([a-z])([A-Z])/g, '$1 $2')
     .replace(/([A-Z])([a-z])/g, ' $1$2')
     .replace(/ +/g, ' ')
     .replace(/^./, str => str.toUpperCase())
     .trim();
  • The above function takes one argument labelName, and returns it after applying some modifications on that string
  1.  static englishMissingTermGenerator(namespace, locale, declarationName, propertyName) {
         const firstPart = propertyName ? propertyName + ' of the' : '';
         return camelCaseToSentence(firstPart + declarationName);
     }
  • The above function takes 4 arguments, and then returns firstPart + declarationName to make it in camel case in a sentence, so in a way, both the function are doing the same thing, which causes us more overhead therefore there is a need for combining the normalizeLabel logic in englishMissingTermGenerator, but my questions/doubts here are:-
  • Do I need to make changes in englishMissingTermGenerator or in camelCaseToSentence since it is the function that takes one argument like normalizeLabel and apply some regex functionalities, and then returns it.
  • The second question is also in the same path that is englishMissingTermGenerator return something on the condition of whether or not propertyName is present, but that is not the case with normalizeLabel, so do I need to combine both the things inside englishMissingTermGenerator on the condition of if propertyName is present or not something like below:
  • static englishMissingTermGenerator(namespace, locale, declarationName, propertyName) {
        if(propertyName){
          const firstPart = propertyName ? propertyName + ' of the' : '';
          return camelCaseToSentence(firstPart + declarationName);
       }
       else{
          logic of `normalizeLabel` function
              }
      }
  • Also import { englishMissingTermGenerator } from '@accordproject/concerto-vocabulary'; is it the correct way of importing englishMissingTermGenerator functionalities in ui-concerto

Please let me know if I have not understood anything correctly or any suggestions, your help would be greatly appreciated :):

@mttrbrts
Copy link
Member Author

In my understanding both normalizeLabel and camelCaseToSentence are trying to do the same thing, except that normalizeLabel properly handles sequential capital letters, e.g. compare the output of f("customerSSN").

I say, that replacing the implementation of camelCaseToSentence with that from normalizeLabel and then exposing camelCaseToSentence in the VocabularyManager API would solve the problem.

Does that make sense?

@Jeevan-Kiran-Lenka
Copy link

camelCaseToSentence results Customer S S N whereas normalizeLabel produces Customer SSN for the input of f("customerSSN")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty: Starter Good First Issue :octocat: Good for newcomers Type: Enhancement ✨ Improvement to process or efficiency Type: Feature Request 🛍️ New feature or request
Projects
None yet
2 participants