nus-cs2103-AY2425S1 / pe-dev-response

0 stars 0 forks source link

Allowed to assign a produce twice #26

Open nus-se-script opened 21 hours ago

nus-se-script commented 21 hours ago

Steps to reproduce: run command: assign pr/hakka noodles su/Fresh Farms Ltd image.png

run command again: assign pr/hakka noodles su/Fresh Farms Ltd image.png

expected: error message to say product already assigned

actual: allowed to assign again


[original: nus-cs2103-AY2425S1/pe-interim#10] [original labels: severity.Low type.FunctionalityBug]

yu7ong commented 3 hours ago

Team's Response

Justification for Duplicates:

The defect lies in the assignProductToSupplier method of AssignProductCommand class. Specifically, the issue arises from adding the product to the supplier's productList before updating the product's supplierName. This inconsistency creates two problematic behaviors:

We attempted to fix:

In the original implementation, the line: updatedProductList.add(product); occurs before updating the supplierName of the product:

private void assignProductToSupplier(Model model, Supplier supplier, Product product) {
        Set<Product> updatedProductList = new HashSet<>(supplier.getProducts());
        updatedProductList.add(product); //problematic line 

        Product updatedProduct = new Product(product.getName(), product.getStockLevel(), product.getTags());
        updatedProduct.setSupplierName(supplier.getName());

        Supplier updatedSupplier = new Supplier(
                supplier.getName(), supplier.getPhone(), supplier.getEmail(),
                supplier.getAddress(), supplier.getTags(), updatedProductList);

        model.setProduct(product, updatedProduct);
        model.setSupplier(supplier, updatedSupplier);
    }

After the fix:

    private void assignProductToSupplier(Model model, Supplier supplier, Product product) {
        Set<Product> updatedProductList = new HashSet<>(supplier.getProducts());

        Product updatedProduct = new Product(product.getName(), product.getStockLevel(), product.getTags());
        updatedProduct.setSupplierName(supplier.getName());

        updatedProductList.add(updatedProduct); //after fix 
        Supplier updatedSupplier = new Supplier(
                supplier.getName(), supplier.getPhone(), supplier.getEmail(),
                supplier.getAddress(), supplier.getTags(), updatedProductList);

        model.setProduct(product, updatedProduct);
        model.setSupplier(supplier, updatedSupplier);
    }

The supplierName is updated before the product is added to the supplier's list. This ensures that checks in checkProductAssignmentStatus and findSupplierWithProduct behave correctly.

The bugs meet the duplicate definition because they are both caused by the same underlying defect in the assignProductToSupplier method. Fixing this defect addresses both issues simultaneously, confirming that they cannot be fixed independently.

Duplicate status (if any):

Duplicate of #3710