Personas with no name show as null on visualisations (LLC-57) #293
Conversation
There was a problem hiding this comment.
One little comment on code duplication @PrinceWaune but this is good 👍 No need to change in this PR.
| @@ -18,4 +19,16 @@ describe('createPersona', () => { | |||
| assert.equal(actualPersona.name, 'Dave'); | |||
| assert.equal(actualPersona.organisation, TEST_ORGANISATION); | |||
| }); | |||
|
|
|||
| it('Should create a persona with personaId instead name, if name not exists.', async () => { | |||
| name: opts.name, | ||
| organisation: new ObjectID(opts.organisation), | ||
| }; | ||
| const personaId = new ObjectID(); |
There was a problem hiding this comment.
Generating this ahead of time and using it as the name is a smart solution, nicely done 👍
There was a problem hiding this comment.
@ryansmith94 I'm not sure it's a good idea to drop the id into the name field on the db. I feel a name field should hold the name, and the change should be just done in the ui?
There was a problem hiding this comment.
Yeah I totally agree, If I remember correctly we had discussed that as the preferred option but I think it was difficult to implement as it would require changes to the pipelines when personas were chosen.
|
|
||
| // Formats the persona to be returned. | ||
| const persona: Persona = { | ||
| id: opResult.insertedId.toString(), | ||
| name: opts.name, | ||
| name: Boolean(opts.name) ? opts.name : personaId.toString(), |
There was a problem hiding this comment.
In future, we should try to use a variable instead to avoid code duplication (line 24 and line 17).
| @@ -1,7 +1,7 @@ | |||
| // tslint:disable-next-line:no-unused | |||
| import PersonaModel from '../../models/Persona'; | |||
| import service from '../../tester'; | |||
| import { TEST_ORGANISATION } from '../utils/values'; | |||
| import { TEST_ORGANISATION } from './values'; | |||
|
🎉 This PR is included in version 1.7.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
@PrinceWaune assuming automatic releases still work as expected here, this will now create a release and Semantic Release will comment on this issue when that is done with the version number. Please use that version to update the package.json and yarn.lock in your Enterprise PR 👍 |
|
Great! Thanks for the release! |
Closed