Skip to content

Comments

Added postfix for repeatedly added Cohort Definition & Concept Set#1122

Closed
aklochkova wants to merge 6 commits intomasterfrom
issue-1100-broken-import-plp-ple-cc-pw
Closed

Added postfix for repeatedly added Cohort Definition & Concept Set#1122
aklochkova wants to merge 6 commits intomasterfrom
issue-1100-broken-import-plp-ple-cc-pw

Conversation

@aklochkova
Copy link
Contributor

Fixes #1100

@aklochkova aklochkova requested review from anthonysena and wivern May 23, 2019 10:13
Copy link
Contributor

@pavgra pavgra left a comment

Choose a reason for hiding this comment

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

What about regular copy methods?
image

@aklochkova aklochkova requested a review from pavgra May 27, 2019 18:15
@anthonysena anthonysena added this to the v2.7.2 milestone May 28, 2019

public static String getNameWithSuffix(String dtoName, Function<String, Integer> countLikeName){
while (countLikeName.apply(dtoName) > 0){
dtoName = dtoName + " (" + countLikeName.apply(dtoName) + ")";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you query database twice?


public static String getNameForCopy(String dtoName, Function<String, Integer> countLikeName, Optional<?> existingObject) {
String nameWithPrefix = String.format(ENTITY_COPY_PREFIX, dtoName);
int similar = countLikeName.apply(nameWithPrefix);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you duplicate logic and do not re-use getNameWithSuffix?

}

@GET
@Path("/{id}/copyName")
Copy link
Contributor

Choose a reason for hiding this comment

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

Standard practice for URL names is to use hyphens

}

public static String getNameWithSuffix(String dtoName, Function<String, Integer> countLikeName){
while (countLikeName.apply(dtoName) > 0){
Copy link
Contributor

Choose a reason for hiding this comment

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

while?

@pavgra
Copy link
Contributor

pavgra commented May 29, 2019

Test:

  1. Create cohort named COPY OF: abcde
  2. Create cohort named abc
  3. Copy cohort named abc

Expected result:
COPY OF: abc

Actual result:
COPY OF: abc (1)

@pavgra
Copy link
Contributor

pavgra commented May 29, 2019

Test:

  1. Create abcde
  2. Create COPY OF: abcde (1)
  3. Copy abcde

Expected result:
COPY OF: abcde (2)

Actual result:
Frozen Atlas and exception in console
image

@pavgra
Copy link
Contributor

pavgra commented May 30, 2019

Closed in favor of #1145 which resolves all the issues

@pavgra pavgra closed this May 30, 2019
@pavgra pavgra deleted the issue-1100-broken-import-plp-ple-cc-pw branch May 30, 2019 00:17
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.

Broken import of PLP, PLE, CC, Pathway

3 participants