Converted all prices from double to decimal

[MatteoPiovanelli-Laser]

This fixes #86

As I mentioned in the comments there, I did not find a way to migrate the DiscountPrice column of the ProductPartRecord table, because setting a default value creates a constraint I was unable to treat correctly.

[MatteoPiovanelli-Laser]

@armanforghani Does this solve the issue you opened?

[armanforghani]

@MatteoPiovanelli-Laser My issue was about all columns including DiscountPrice column.

[MatteoPiovanelli-Laser]

@armanforghani I see that. However, there currently is not a way in Orchard to alter a column where a default constraint is being applied (see https://github.com/OrchardCMS/Orchard/issues/7615). You cannot even drop that column.

I have an SQL script that can be run in MsSql to drop the constraints on DiscountPrice. Note that this will drop ALL DEFAULT CONSTRAINTS from the column, and it's only been tested once on my local machine so far, so there is no guarantee it will work:

DECLARE @sql NVARCHAR(MAX)

WHILE 1=1
BEGIN
    SELECT
        TOP 1  @sql = N'ALTER TABLE Nwazet_Commerce_ProductPartRecord DROP CONSTRAINT ['+constraints.name+N']'
    FROM
        sys.default_constraints constraints
        inner join sys.tables tabs on constraints.parent_object_id = tabs.object_id
        inner join sys.columns cols on cols.column_id = constraints.parent_column_id
    WHERE
        tabs.name = 'Nwazet_Commerce_ProductPartRecord'
        and cols.name = 'DiscountPrice'

    IF @@ROWCOUNT = 0 BREAK

    EXEC (@sql)
END

I did not put this in the migrations, because I have no idea whether this will work for every dbms (I suspect it may not).

The alternative we may currently have is to drop the table entirely and recreate it. This seems like a bit of an overkill.

[bleroy]

I'm not comfortable merging that until there's a good migration path that doesn't require running a T-SQL script. Could we create a whole new column, migrate the existing data into that, then drop the old column?

[MatteoPiovanelli-Laser]

@bleroy that was my plan as well. However, because of the default constraint, the DropColumn command fails.

I'll have to check If I am allowed to drop the whole table without errors/exceptions. If that is possible, I could copy the entirety of it to memory, or to a temporary file. Or even to a "clone" table, that I would rename after dropping the original one.

[bleroy]

Does it rollback the transaction when dropping the column fails? I'm thinking, worst case, we leave it there.

[MatteoPiovanelli-Laser]

I have tried, and I can drop the whole table. However, I didn't do the whole process of copying the table to memory and then over to the new db table (I attempted to use IRepository, bu I messed up somehow).

In light of #112, do we even want to do this migration to this table right now? Maybe it would make more sense to just migrate the thing to a VersionRecord. Price is definitely one of the properties that should be versioned.

[bleroy]

Yeah, that sounds fine to me. I'll merge when conflicts are fixed.

[MatteoPiovanelli-Laser]

Sorry, but I don't understand what sounds fine to you. I mean, should I remove the migrations for the ProductPartRecord table? Or make ProductPartRecord a VersionRecord (and make sure everything works as it does now)?

In the second case, I would have to make, I think, some changes around to ensure that wherever we Get products, we are getting the version we want, and that some properties' changes affect all versions (e.g.: if I change inventory in a draft that should also change the inventory in the opublished version). This is somewhat similar to the CultureNeutral setting that we pushed to Orchard's dev branch with the LocalizationPR.

[bleroy]

If we think the price should be versionable like we do, then the product part record should be a version record.

[MatteoPiovanelli-Laser]

ok. I like that. I feel like I should do that (ProductVersionRecord) in this PR here, so that we don't have further migrations for converting to decimal. Does that sound ok?

[bleroy]

Sure.

[MatteoPiovanelli-Laser]

Merged things in.

Currently I removed the migration for the ProductPart, because I still have to do that versioning thing

[MatteoPiovanelli-Laser]

The way I am implementing this, seeing also #112 is:

• ProductPartVersionRecord is the record for ProductPart. THe price properties there are already decimal type.
• The sku gets no special treatment by default. It can be changed in a draft, and then published normally. We may wish to add some special considerations about the sku in teh features for advaned SKU management, but I'll have to think some more on meaningful scenarios.
• The inventory is kept in synch between draft and published versions.

Regarding the inventory: I am testing things out by copying the Inventory property between parts, but it'd be cleaner to use the IProductInventoryService we are introducing in #113