Rename `visible_in_bulk_form` field to `is_private`

Question

Rename `visible_in_bulk_form` field to `is_private`

Closed this issue a year ago · 7 comments

We need to standardize the visible_in_bulk_form field on the product model in a way that it can be utilized on multiple platforms. Currently, This field defines if a product can be seen in the bulk enrollment form or not.

This will enable the finance team as well in identifying the courses in Public/Private manner. So, adding a field(boolean) Private would enable us to make the products public or private. The default

Acceptance Criteria:

Rename this field to is_private, And refactor all of its usages to keep things running just like before. The default value of this boolean would be False, Which means the product would be public by default.
Replicate this new change in BI reports as well.

Answer 1 · 2023-08-08T11:57:32.000Z

@arslanashraf7 I thought you preferred to add a new field instead of renaming the existing one. Did you change your mind? I agree sticking to one field will be clearer, but I'm concerned about the complexity of going from visible_in_bulk_form, where True implies that the course is public, to is_private, which has the opposite meaning.

In any case, since this is a Boolean, I prefer the convention of starting with is, so I updated the description to is_private.

Answer 2 · 2023-08-08T12:53:27.000Z

@arslanashraf7 I thought you preferred to add a new field instead of renaming the existing one. Did you change your mind? I agree sticking to one field will be clearer, but I'm concerned about the complexity of going from visible_in_bulk_form, where True implies that the course is public, to is_private, which has the opposite meaning.

In any case, since this is a Boolean, I prefer the convention of starting with is, so I updated the description to is_private.

@pdpinch I remember discussing adding a new field but then left that idea because that would increase ambiguity in managing so many fields e.g. visible_in_bulk_form , is_external and (is_private - Being a new field).

Also, Since visible_in_bulk_form and is_private were going to have the same functionality so i think we decided to just rename it.

but I'm concerned about the complexity of going from visible_in_bulk_form, where True implies that the course is public, to is_private, which has the opposite meaning.

We can manage this in this way:

Rename the field and invert the existing values for visible_in_bulk_form in DB in a single migration
There are a total of 4 places where it's used and we can make those depend upon is_private easily.

However, If you think we should go for a new field, That is still a feasible solution as well. Let me know what you think.

Answer 3 · 2023-08-08T13:09:30.000Z

I just stumbled onto an interesting case that I think is worth discussing a bit before we decide this question of one field or two.

https://xpro.mit.edu/admin/ecommerce/product/516/

This is an external course from Emiritus, which the course team considers "public."

But, as an external course, it should not appear in the bulk purchase form at https://xpro.mit.edu/ecommerce/bulk/

I noticed today that it has visible_in_bulk_form set to True. But, as far as I can tell, it does not display in https://xpro.mit.edu/ecommerce/bulk/

Is there additional logic in the bulk purchase form that omits external courses? This would make sense to me, but it makes the current behavior of visible_in_bulk_form ambiguous.

Answer 4 · 2023-08-08T13:19:47.000Z

Is there additional logic in the bulk purchase form that omits external courses? This would make sense to me, but it makes the current behavior of visible_in_bulk_form ambiguous.

Yes, The bulk form calls /products API. That API filters(Excludes) the external courses and programs. Here is the logic behind that.

Answer 5 · 2023-08-08T13:47:46.000Z

Adding to the previous comment:

The /products API works in a reusable manner in multiple places.
So, We can say that visible_in_bulk_form won't have any effect on External products because they are never going to be a part of /products API.
Also, The logic that checks which products (out of all products from /products API) to display to the user on the bulk form is here on the frontend.

Answer 6 · 2023-08-10T14:47:20.000Z

Oh boy. So we have logic in two different places.

For the sake of this issue, it seems fine to rename the field from visible_in_bulk_form to is_private and flip the values.

But to maintain the function of the bulk form, do we change the products API, or the frontend code? Note that "private" products still need to be available on the checkout page.

Answer 7 · 2023-08-10T14:53:24.000Z

Oh boy. So we have logic in two different places.

For the sake of this issue, it seems fine to rename the field from visible_in_bulk_form to is_private and flip the values.

But to maintain the function of the bulk form, do we change the products API, or the frontend code? Note that "private" products still need to be available on the checkout page.

I think just changing the field name and flipping values should be all that needs to be done, So the things will keep working as before. e.g. Public products will only be available in the bulk form and the private+public will be available for checkout because checkout doesn't care for visible_in_bulk_form field, Only bulk form does.