Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(refactor):03-4505-Refactor stock operations forms for ease of maintenance #258

Open
wants to merge 37 commits into
base: main
Choose a base branch
from

Conversation

Omoshlawi
Copy link

@Omoshlawi Omoshlawi commented Feb 24, 2025

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

  1. Responsible person defaulted to current session user when configured
autofill-responsible.person.mp4
  1. Responsible person other bugout on submit and other appearing multiple times
responsible-person-other.mp4
  1. Responsible person other not properly autofeeling on launch form in edit mode
resp0nsible-person-other-edit-mode.mp4
  1. When updating stock operation , if you add stock operation item it bugged out
update-operation-add-items.mp4
  1. When updating stock operation, if you delete stock operation items it bugged out
update-stock-operation-delete-items.mp4
  1. UI enhancements
  2. Code refactor to address the following issues

** Component Coupling Issues: **

  • High interdependency between components causing cascading effects
  • Changes in one component frequently trigger unintended side effects in others
  • Bug fixes often introduce new issues in seemingly unrelated areas
  • Complex dependency chains making code changes high-risk

** State management issues: **

  • Widespread use of mutable state patterns leading to unpredictable behavior
  • Direct state references passed between components allowing uncontrolled modifications
  • Multiple components modifying shared state in memory without proper tracking
  • Lack of reactive updates when state changes occur
  • No clear state ownership or update patterns

** Unused code segments **
-Inconsistent UI patterns affecting user experience

Screenshots

Related Issue

Other

…ce base operation detail form by making conditional rendering of fields and ensuring autofeeling
…ce to empty state add buton and launching well
…ockOperationItemCell, enhance StockAvailability with error handling and loading states
…ement in stock operations;seperate concerns on hook
…nd improve styling; refactor stock operation issue button
…user selectors; improve mock implementations
… operation form with additional tab functionality
@@ -24,15 +24,11 @@ interface BatchNoSelectorProps<T> {

const BatchNoSelector = <T,>(props: BatchNoSelectorProps<T>) => {
const { isLoading, stockItemBatchNos } = useStockItemBatchNos(props.stockItemUuid);
console.log('stockItemBatchNos', stockItemBatchNos);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove this console log

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok,sure.Its even an older copy which is used to derive the improved one hence it unused and forgot to remove it.all cleaned up now

@jabahum
Copy link
Collaborator

jabahum commented Mar 5, 2025

@makombe

@@ -0,0 +1,39 @@
import React from 'react';

export const EmptyDataIllustration = ({ width = '64', height = '64' }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Omoshlawi i am thiinking we use empty state from framework instead of duplicating efforts

handleAdd?: () => void;
message: string;
};
const EmptyState: React.FC<Props> = ({ headerTitle, handleAdd, message }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as here

const response: FetchResponse<StockOperationDTO> = await (isEditing ? updateStockOperation : createStockOperation)(
payload,
);
// const response: FetchResponse<StockOperationDTO> = await (isEditing ? updateStockOperation : createStockOperation)(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incase this is not required anymore we can remove it making sure its alternative implementation works as was before

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants