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

[#5080] Added active state to fix indicator approval workflow #5084

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ifirmawan
Copy link
Contributor

@ifirmawan ifirmawan commented Aug 23, 2022

TODO / Done

Summarize what has been changed / what has to be done in order to finalize the PR.

  • Update status description.
  • Add new icon history.
  • Create audit trail modal
  • Call /indicator_period_data_framework/ by period

Test plan

What tests are necessary to ensure this works or doesn't break anything working

  • View history at reporting period

View history at reporting period

  • Login with the MnE account
  • Go to project id : 9604.
  • Move to the results admin tab.
  • Find active indicators and click the clock history icon.

@ifirmawan ifirmawan self-assigned this Aug 23, 2022
@ifirmawan ifirmawan linked an issue Aug 23, 2022 that may be closed by this pull request
@coveralls
Copy link
Collaborator

coveralls commented Aug 24, 2022

Coverage Status

Coverage remained the same at 66.732% when pulling 0519265 on 5080-feature-request-nuffic-indicator-approval-workflow into c9d073b on master.

@ifirmawan ifirmawan force-pushed the 5080-feature-request-nuffic-indicator-approval-workflow branch from a972475 to 4a5946b Compare August 26, 2022 13:07
@ifirmawan ifirmawan changed the title [#5080] Update status description [#5080] Added active state to fix indicator approval workflow Aug 26, 2022
@ifirmawan ifirmawan force-pushed the 5080-feature-request-nuffic-indicator-approval-workflow branch 2 times, most recently from 6e9e9f5 to 11f3ef3 Compare August 29, 2022 05:12
@ifirmawan ifirmawan requested a review from AkvoAnt August 29, 2022 05:15
@ifirmawan ifirmawan marked this pull request as ready for review August 29, 2022 05:31
@ifirmawan ifirmawan requested a review from navins94 August 29, 2022 07:15
import approvedSvg from '../images/status-approved.svg'
import pendingSvg from '../images/status-pending.svg'
import revisionSvg from '../images/status-revision.svg'
import { DeclinePopup } from './DeclinePopup'

const Aux = node => node.children

export const StatusPeriod = ({ update, pinned, index, handleUpdateStatus, t }) => {
export const StatusPeriod = ({ update, pinned, index, handleUpdateStatus }) => {
const { t } = useTranslation()
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's better to have descriptive name instead of t

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted, Thank you Navin. Fixing in progress

<Col lg={22} md={22} sm={24} xs={24}>
{isEmpty(period) && (
<div className="period-caption">
{moment(item?.period?.periodStart, 'DD/MM/YYYY').format('DD MMM YYYY')} - {moment(item?.period?.periodEnd, 'DD/MM/YYYY').format('DD MMM YYYY')}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice to create variable and assign values and render variable. If later on we convert it to component it would be easier to just pass the variable instead

)}
>
<div className="period-caption">
{`${moment(period?.periodStart, 'DD/MM/YYYY').format('DD MMM YYYY')} - ${moment(period?.periodEnd, 'DD/MM/YYYY').format('DD MMM YYYY')}`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice to create variable and assign values and render variable. If later on we convert it to component it would be easier to just pass the variable instead

@ifirmawan ifirmawan force-pushed the 5080-feature-request-nuffic-indicator-approval-workflow branch from 11f3ef3 to b827e0d Compare August 30, 2022 01:13
@ifirmawan ifirmawan requested a review from navins94 August 30, 2022 01:16
@ifirmawan ifirmawan force-pushed the 5080-feature-request-nuffic-indicator-approval-workflow branch from b827e0d to 4ee4908 Compare August 30, 2022 11:48
[#5051] Make audit trail components responsive in mobile view
Active status helps MnE managers to identify updates of indicators that have an unlocked period still in progress.
the following what has to be done in order to finalize  this feature :
1. Update status description.
2. Add new icon history.
3. Create audit trail modal
4. Call /indicator_period_data_framework/ by period
   Pulling data audit trails from endpoint /indicator_period_data_framework/ by period ID of selected update.
@ifirmawan ifirmawan force-pushed the 5080-feature-request-nuffic-indicator-approval-workflow branch from 4ee4908 to 4c408fd Compare August 30, 2022 11:52
- Reduce time column size.
- Add new constant variable for status terminology.
@ifirmawan ifirmawan force-pushed the 5080-feature-request-nuffic-indicator-approval-workflow branch from 4c408fd to 0519265 Compare August 30, 2022 13:30
@ifirmawan ifirmawan removed the request for review from navins94 August 31, 2022 13:17
@MichaelAkvo
Copy link
Contributor

Adding link to #5080 for Zenhub

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

Successfully merging this pull request may close these issues.

Feature Request: Nuffic - Indicator approval workflow
4 participants