PM QA 2.58 fix #3253
PM QA 2.58 fix #3253
Conversation
- update button text
- fixed crash at learning download
- fixed sync issue
|
@damagatchi retest this please |
1 similar comment
|
@damagatchi retest this please |
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
this has been flagged repeatedly and think we should really create separate PRs instead of clubbing them all together even if they seem small. This really inhibit out ability to quickly rollback any changes plus also blocks merge on other simple issue to get merged quickly.
I think it's fine to club the one line changes in a single PR but that should be added in separate commits atleast. Here can you remove the changes in app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java and app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java into different individual PRs for their respective individiual bug fixes.
| jobCard.tvViewMore.setOnClickListener(v -> Navigation.findNavController(v) | ||
| .navigate( | ||
| ConnectLearningProgressFragmentDirections.actionConnectJobLearningProgressFragmentToConnectJobDetailBottomSheetDialogFragment())); |
There was a problem hiding this comment.
formatting seems wrong, also abstract this as a navigateToX method.
| jobList.addAll(reviewLearnApps); | ||
| jobList.addAll(finishedItems); | ||
| initRecyclerView(); | ||
| Collections.sort(learnApps, (job1, job2) -> job1.getLastAccessed().compareTo(job2.getLastAccessed())); |
There was a problem hiding this comment.
formatting changes should not be part of a commit that's making functional changes, it makes these PRs much more difficiult to review than needed.
There was a problem hiding this comment.
okay noted usually do formatting at same time so don't miss at the end
| }, getViewLifecycleOwner(), Lifecycle.State.RESUMED); | ||
|
|
||
| refreshData(); | ||
| if (!requireActivity().isFinishing()) { |
There was a problem hiding this comment.
why would activity be finishing here ?
There was a problem hiding this comment.
it got finish here but the root cause why this fragment is called again is still not known i have tried to debug connect activity is not called but still this fragment gets called
There was a problem hiding this comment.
@pm-dimagi Can you please tag ticket here for the bug you mentioned?
There was a problem hiding this comment.
| FirebaseAnalyticsUtil.reportConnectTabChange(tabLayout.getTabAt(position).getText().toString()); | ||
| View view = viewPagerAdapter.createFragment(position).getView(); | ||
| if (view != null) { | ||
| binding.connectDeliveryProgressViewPager.getLayoutParams().height = |
There was a problem hiding this comment.
I don't really get what bug is this trying to solve, Don't think we should be tryign to set the height on view pager manually ever.
There was a problem hiding this comment.
This is the part of this merge I think you and dave has made #2989 discussed here the bug trying to solve here is https://dimagi.atlassian.net/browse/QA-7923
There was a problem hiding this comment.
I think that was only a short term fix from Dave just to unblock a cohort and now that this is no longer part of the codebase we should look for a better fix here.
|
solved the issue in different PR |
Product Description
https://dimagi.atlassian.net/browse/QA-7923
Fix-> This code got removed during the refactoring which got earlier removed .. was a part of this merge
QA-7926
Fix-> There was a return statement in if else condition removed that in updateButtons function
QA-7927
Fix-> put the visibility condition
https://dimagi.atlassian.net/browse/QA-7929
Fix-> there was no code for this put the appropiate code
QA-7932
Fix-> added requireActivity.isFinishing() as we have called activity.finish but before that fragment is calling reefreshData
QA-7921
Fix_ called sync code which was not present earlier
Technical Summary
Feature Flag
Safety Assurance
Safety story
Automated test coverage
QA Plan
Labels and Review