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

Update billable services workspace #267

Merged

Conversation

agesa3
Copy link
Contributor

@agesa3 agesa3 commented Jul 9, 2024

Requirements

  • This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.
  • My work conforms to the OpenMRS 3.0 Styleguide.
  • I checked for feature overlap with existing widgets.

Summary

Screenshots

Screen.Recording.2024-07-25.at.16.24.51.mov

None.

Related Issue

None.

Other

None.

@agesa3 agesa3 marked this pull request as draft July 9, 2024 12:27
@agesa3 agesa3 marked this pull request as ready for review July 17, 2024 06:53
@ojwanganto
Copy link
Contributor

@agesa3 could you please attach a video showing what this PR does? Thanks

@agesa3
Copy link
Contributor Author

agesa3 commented Jul 18, 2024

@agesa3 could you please attach a video showing what this PR does? Thanks

@agesa3 could you please attach a video showing what this PR does? Thanks

I've attached a video but still there is an issue with the backend API when it comes to updating I had informed @Injiri about it and he was to action on it since the API creates a new entry rather than updating.If that is sorted out the code will be fully ready since on the frontend everything seems to be ok.

@ojwanganto
Copy link
Contributor

@agesa3 - You may have forgotten to upload the video. Also, kindly resolve the conficts

@agesa3
Copy link
Contributor Author

agesa3 commented Jul 19, 2024

@agesa3 - You may have forgotten to upload the video. Also, kindly resolve the conficts

Actioning on it

@ojwanganto
Copy link
Contributor

Thanks, @agesa3. Could you please look at the styling for the edit form? The margins don't look OK.

@agesa3
Copy link
Contributor Author

agesa3 commented Jul 23, 2024

Thanks, @agesa3. Could you please look at the styling for the edit form? The margins don't look OK.

Done.Updated the styling and video

Thanks, @agesa3. Could you please look at the styling for the edit form? The margins don't look OK.

Done and updated the video.

@donaldkibet any guidance on this .In my local the import works ok but here on CI it seems the import of WorkspaceContainer is not found.
Screenshot 2024-07-23 at 11 24 09

donaldkibet
donaldkibet previously approved these changes Jul 23, 2024
Copy link
Contributor

@donaldkibet donaldkibet left a comment

Choose a reason for hiding this comment

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

Thanks @agesa3

@@ -37,6 +46,12 @@ const BillableServices = () => {
const [showOverlay, setShowOverlay] = useState(false);
const [overlayHeader, setOverlayTitle] = useState('');

const handleEditClick = (initialData) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have a type for the initialData and probably update the name to something close e.g service

@donaldkibet
Copy link
Contributor

Thanks, @agesa3. Could you please look at the styling for the edit form? The margins don't look OK.

Done.Updated the styling and video

Thanks, @agesa3. Could you please look at the styling for the edit form? The margins don't look OK.

Done and updated the video.

@donaldkibet any guidance on this .In my local the import works ok but here on CI it seems the import of WorkspaceContainer is not found. Screenshot 2024-07-23 at 11 24 09

We should update to the latest version of @openmrs/esm-framework to fix this error

@ojwanganto
Copy link
Contributor

@agesa3 - thanks for the updates. You will at least need to refine styling of the form. Kindly ping the team once you have something to be reviewed.

@agesa3
Copy link
Contributor Author

agesa3 commented Jul 25, 2024

@agesa3 - thanks for the updates. You will at least need to refine styling of the form. Kindly ping the team once you have something to be reviewed.

@ojwanganto sorry had uploaded the wrong video.Ive corrected and uploaded the correct one

@ojwanganto
Copy link
Contributor

@agesa3 are you available to complete on this task? We need it urgently

@agesa3
Copy link
Contributor Author

agesa3 commented Aug 1, 2024

@agesa3 are you available to complete on this task? We need it urgently

I completed it as at last week not unless there is any update that I need to make.Ive noticed there is some conflict and am resolving it now.

@ojwanganto
Copy link
Contributor

@agesa3 - so is the problem an issue with the backend?

@agesa3
Copy link
Contributor Author

agesa3 commented Aug 1, 2024

@agesa3 - so is the problem an issue with the backend?

Yes, Simon had created a fix and it was merged and it worked ok but then a few days after that it started failing and whenever an update is made it creates a new entry rather than updating.

@donaldkibet donaldkibet force-pushed the update-billable-services-workspace branch from fbc80b9 to 527ea91 Compare August 5, 2024 10:08
@donaldkibet donaldkibet merged commit 5b81cb1 into palladiumkenya:main Aug 5, 2024
7 checks passed
donaldkibet added a commit that referenced this pull request Aug 6, 2024
donaldkibet added a commit that referenced this pull request Aug 6, 2024
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.

3 participants