Shaharsol/eco 1574/tranlations csv by title #370
Conversation
modified the sample csv
Codecov Report
@@ Coverage Diff @@
## master #370 +/- ##
=======================================
Coverage 58.05% 58.05%
=======================================
Files 50 50
Lines 3011 3011
Branches 444 444
=======================================
Hits 1748 1748
Misses 1258 1258
Partials 5 5Continue to review full report at Codecov.
|
| @@ -61,7 +68,8 @@ const EXCLUDED = [ | |||
| ]; | |||
|
|
|||
| function constructRow(contentType: ContentType, key: string, str: string) { | |||
| const path = `${ contentType }:${ key.replace(KEY_TO_PATH_REGEX, "$1") }`; | |||
| const path = `${ contentType }:${ key.replace(/:(.*?):/, ":").replace(/\[.\]/g, "") }`; | |||
There was a problem hiding this comment.
Move it out to a constant, it helps document your code.
There was a problem hiding this comment.
added a comment instead, much more useful
scripts/src/admin/translations.ts
Outdated
| @@ -61,7 +68,8 @@ const EXCLUDED = [ | |||
| ]; | |||
|
|
|||
| function constructRow(contentType: ContentType, key: string, str: string) { | |||
| const path = `${ contentType }:${ key.replace(KEY_TO_PATH_REGEX, "$1") }`; | |||
| const path = `${ contentType }:${ key.replace(/:(.*?):/, ":").replace(/\[.\]/g, "") }`; | |||
|
|
|||
| @@ -123,18 +131,19 @@ async function getCsvRowData() { | |||
| let rows: CsvRow[] = []; | |||
| allOffers.forEach(offer => { | |||
| const offerId = offer.id; | |||
| const offerName = offer.name; | |||
| const offerContent: OfferContent = allContent.filter(obj => obj.offerId === offerId)[0]; | |||
There was a problem hiding this comment.
You can change this to check for name and remove the offerId var completely.
There was a problem hiding this comment.
much more clear to use offerId as pkey
scripts/src/admin/translations.ts
Outdated
| @@ -9,6 +9,8 @@ import { ContentType, Offer, OfferContent } from "../models/offers"; | |||
| import { OfferTranslation } from "../models/translations"; | |||
| import { path } from "../utils/path"; | |||
|
|
|||
| import * as _ from "lodash"; | |||
There was a problem hiding this comment.
I really don't see why you need this, please reconsider.
scripts/src/admin/translations.ts
Outdated
| console.error("content eval failed: \neval string: %s\n error message: %s", evalString, e); | ||
| // const [table, offerId, column, jsonPath] = getCsvKeyElements(csvKey); | ||
| const [table, offerName, column, jsonPath] = getCsvKeyElements(csvKey); | ||
| const currentOffer = _.find(allOffers, function(offer: Offer) { |
There was a problem hiding this comment.
Why lodash and not just allOffers.find()?
If you are doing this just to get the offer id in order to get to the offer content, you can just do one query and join the two tables together.
There was a problem hiding this comment.
Also use arrow functions instead of function definistions.
There was a problem hiding this comment.
using js find instead. hard to get used to not using _
partial implementation of the suggested in the ticket. Didn't go as far as changing the db schema, just made sure template file read/write with be with offer.name as part of the key instead of offer.id. db offer_content and offer_content_translation still use offer_id