O3-5662: Migrate SequentialReceiptNumberGeneratorService to OpenMRS Service

[EDSONZ-WASSWA]

Summary

This Pr now migrates SequentialReceiptNumberGeneratorService to the standard
OpenMRS service pattern, I tried to be consistent with how other services in this
module are structured.

Changes Made

Interface (SequentialReceiptNumberGeneratorService)

• Renamed from ISequentialReceiptNumberGeneratorService — drops
  the I prefix per standard Java and OpenMRS naming conventions
• Now extends OpenmrsService instead of the legacy
  IObjectDataService<SequentialReceiptNumberGeneratorModel>
• Removed methods inherited from the old generic layer
  (setRepository, getAll, purge) that do not belong on a
  purpose-built service interface
• Added save(SequentialReceiptNumberGeneratorModel) as an explicit,
  documented method with proper @throws Javadoc
• Fixed two incorrect @should Javadoc tags that referenced
  NullPointerException — changed to IllegalArgumentException

Implementation (SequentialReceiptNumberGeneratorServiceImpl)

• Now extends BaseOpenmrsService instead of the legacy
  BaseObjectDataServiceImpl
• Removed getPrivileges() and validate() overrides that only
  existed to satisfy the old base class
• @Transactional declared at class level; readOnly = true
  overrides applied only to read methods
• All null-argument guards now throw IllegalArgumentException
  consistently — previously saveSequence and purgeSequence
  threw NullPointerException
• Added null guard to save for consistency

Spring Context (moduleApplicationContext.xml)

• Updated serviceContext registration from
  ISequentialReceiptNumberGeneratorService to
  SequentialReceiptNumberGeneratorService

Consumers I Updated

• SequentialReceiptNumberGenerator.java
• AbstractSequentialReceiptNumberGenerator.java
• SequentialReceiptNumberGeneratorController.java
• SequentialReceiptNumberGenerator2xController.java

Tests (SequentialReceiptNumberGeneratorServiceTest)

• Renamed from ISequentialReceiptNumberGeneratorServiceTest
• Changed base class from IObjectDataServiceTest to
  BaseModuleContextTest the old base was tied to the generic
  service layer being replaced
• Service now obtained via Context.getService() in @Before
• Removed orphaned override methods (createEntity,
  updateEntityFields, assertEntity, getTestEntityCount)
  that only existed to satisfy the old test base class
• Fixed two test methods that expected NullPointerException
  now correctly expect IllegalArgumentException
• Removed getOnly_shouldReturnANewModelIfNoneHasBeenDefined
  which depended on service.purge(model) a method no longer
  on the service interface

Tests

All tests pass:

• SequentialReceiptNumberGeneratorServiceTest
• SequentialReceiptNumberGeneratorTest
• All other tests passed

Related Issue

https://openmrs.atlassian.net/browse/O3-5662

please @NethmiRodrigo @wikumChamith please have a look at this PR thank you

[Josephkagimu1]

Hello @EDSONZ-WASSWA thanks alot for working on this , but try fixing some few things here :

The previous IObjectDataServiceTest base class provided CRUD test coverage via createEntity, updateEntityFields, assertEntity, and getTestEntityCount. These have been removed without replacement.
Could you add explicit tests for the new service methods? Something like:
@test
public void save_shouldPersistModel() {
SequentialReceiptNumberGeneratorModel model = new SequentialReceiptNumberGeneratorModel();
// set required fields...
SequentialReceiptNumberGeneratorModel saved = service.save(model);
assertNotNull(saved.getId());
}

@test
public void getAll_shouldReturnSavedModels() {
List results = service.getAll();
assertNotNull(results);
// assert expected count from test dataset
}

@test
public void purge_shouldDeleteModel() {
SequentialReceiptNumberGeneratorModel model = service.getAll().get(0);
service.purge(model);
assertFalse(service.getAll().contains(model));
}

note : This ensures we don't lose coverage for basic persistence operations during the migration.

[EDSONZ-WASSWA]

Hello @EDSONZ-WASSWA thanks alot for working on this , but try fixing some few things here :

The previous IObjectDataServiceTest base class provided CRUD test coverage via createEntity, updateEntityFields, assertEntity, and getTestEntityCount. These have been removed without replacement. Could you add explicit tests for the new service methods? Something like: @test public void save_shouldPersistModel() { SequentialReceiptNumberGeneratorModel model = new SequentialReceiptNumberGeneratorModel(); // set required fields... SequentialReceiptNumberGeneratorModel saved = service.save(model); assertNotNull(saved.getId()); }

@test public void getAll_shouldReturnSavedModels() { List results = service.getAll(); assertNotNull(results); // assert expected count from test dataset }

@test public void purge_shouldDeleteModel() { SequentialReceiptNumberGeneratorModel model = service.getAll().get(0); service.purge(model); assertFalse(service.getAll().contains(model)); }

note : This ensures we don't lose coverage for basic persistence operations during the migration.

Thank you @Josephkagimu1 for the review
The methods already have test methods, you have really mentioned out what i missed,
The save(), getAll(), and purge() methods are leftovers from IObjectDataService and the better option is to remove them completly
I'll do the fix chap chap Thank you

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[EDSONZ-WASSWA]

@Josephkagimu1 I have made update, look at them and see

[Josephkagimu1]

nit: The interface no longer extends IObjectDataService, but this impl still extends BaseObjectDataServiceImpl. This creates an inconsistent inheritance — the impl inherits methods (like getAll(), save()) from the base class and re-declares them on the interface separately.

Consider either:
Replacing extends BaseObjectDataServiceImpl<...> with extends BaseOpenmrsService and injecting the repository directly, or
Keeping the current inheritance but adding a code comment explaining why BaseObjectDataServiceImpl is still needed (e.g., reusing its DAO logic).

[Josephkagimu1]

=> void setRepository(BaseHibernateRepository repository);
This line isnt necessary , removing it would be better , but thanks for the work .

[Josephkagimu1]

Add the save() test after the getOnly_shouldReturnTheFirstModel() test in SequentialReceiptNumberGeneratorServiceTest.java. That's around line ~160.

Add this save test :
@test
public void save_shouldSaveModelSuccessfully() {
SequentialReceiptNumberGeneratorModel model = service.getOnly();
model.setCashierPrefix("UP");
SequentialReceiptNumberGeneratorModel saved = service.save(model);
Context.flushSession();
Assert.assertEquals("UP", saved.getCashierPrefix());
}

Every thing else LGTM . @EDSONZ-WASSWA

[Josephkagimu1]

nit: Consider adding a test for service.save(model) in a follow-up — currently only GroupSequence save is tested, not the model itself.

@ibacher