OBPIH-7459 Create API URL file in e2e project

[alannadolny]

No description provided.

[ewaterman]

What's the advantage of breaking these out into a constant class? I know this is how we do it on the frontend, but if I'm understanding the e2e logic, each of these page URLs are only ever referenced by their matching BasePageModel, so I don't really see a reason to not simply let these URLs be private constants in the page model.

For example: It looks like AUTH_URL is only ever going to be referenced by LoginPage. LoginPage is what gets called from everywhere else in the tests when we want to redirect to that page. This means that AUTH_URL is already abstracted away from the rest of the app so why not let it be a constant in LoginPage? Putting it here just feels like an extra step.

I suppose the argument is that it's nice to have all the URLs together, so if you feel strongly about it feel free to do it this way, I just personally find it more intuitive to have the URL be in the same file as the page.

EDIT: I do see in a few of the tests that these constants are being directly referenced but IMO that just means that those pages should be created as a BasePageModel and the tests refactored to reference that. This is my Java side showing, but I really like how all the page navigation methods in the BasePageModels are strongly typed.

[alannadolny]

I think you're right. Taking into account that we are referencing some of those constants directly, I am not sure what the next steps should be. I like the encapsulation of the POM, but I also like the centralized way of managing URLs

[ewaterman]

if it were up to me, as long as it's not too difficult I'd refactor any test that directly references these values to instead reference some *Page. But since at the moment you and Kasia are the ones who have to work in this code, I think it should be your decision on what structure you like most.

[ewaterman]

These on the other hand I agree should be in a constants file like this because they're used non-uniquely by both pages and services

[ewaterman]

I think "constants" is misleading because the codes are actually dynamically generated based on config.

Instead of /src/constants maybe something like /build? Idk what a good path would be. I'll think about it some more

[alannadolny]

@ewaterman I am moving it to /generated, but if you come up with a better idea, let me know, I will change that

[ewaterman]

This seems incredibly rigid. If I'm understanding, you're trying to build a map like:

[
  "ONE": <productCode>, 
  "TWO": <productCode>, 
  ...
]

Which outputs the ProductCodes file with constants like:

import { Product } from '@/constants/ProductCodes.generated';

productService.getProduct(Product.ONE)

But because this map is based on the product name in the csv, if the structure of the product names change in any way then both this generator any any usage of these constants will error. So actually there isn't much point to making Product be generated since if the name of a product in the csv changes then anywhere in the code that references that product name also needs to change.

So instead of generating it, you might as well just hard code the ProductCodes file to something like:

export const Product = {
  ONE: 1,
  TWO: 2,
  ...
}

then at least if the products in the csv change you only need to change the product code in the ProductCodes file instead of everywhere that references a Product constant.

EDIT: actually now that I think about it, you can still generate the file, just make the "key" always be ONE, TWO, THREE... so that you're not ever changing the field name of the constants

[alannadolny]

I created a map with hardcoded "name: nameFromCsv" so that the keys are stable, and to add something new / adjust the product name, you just have to apply the change here. What do you think about that?

[ewaterman]

I'm fine with this solution.

Expanding on what I meant with my EDIT comment, if you wanted to make it so that even if you change the csv file you don't need to change the code in this generator, you can do something like:

function parseCsv(csv) {
  <loops the csv rows and returns a list of non-null product codes>
}

function buildFileContent(productCodes) {
  if (productCodes.size() < 6) {
    throw new Error("...")
  }
  
  return `// AUTO-GENERATED FILE. DO NOT EDIT.
// Regenerated from src/setup/dataImport/products.csv by scripts/generateProductCodes.mjs
// Output: src/generated/ProductCodes.generated.ts

export const Product = {
  ONE: '${productCodes[0]}',
  TWO: '${productCodes[1]}',
  THREE: '${productCodes[2]}',
  FOUR: '${productCodes[3]}',
  FIVE: '${productCodes[4]}',
  SIX: '${productCodes[5]}',
} as const;

export type ProductCode = (typeof Product)[keyof typeof Product];
`;
}

[ewaterman]

I think this is a fine solution for now, but by always using the same products for all tests, we can get into a situation where one test changes/deletes one of these products, causing all other tests to fail.

A better solution is likely to create the products that we need at the start of each test then delete the products at the end of the test. That way the tests are totally independent of each other.

But we can deal with that later

[alannadolny]

For now, the products from CSV are imported during every single run, so we are sure that those products exist while testing