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

PCHR-4098: Misc BackstopJS fixes and improvements #2818

Merged
merged 4 commits into from
Aug 8, 2018

Conversation

AkA84
Copy link
Contributor

@AkA84 AkA84 commented Aug 6, 2018

This PR contains miscellaneous improvements and fixes to CiviHR's backstopjs set up

Updating to latest stable BackstopJS version

Update BackstopJS to the beta version, currently v3.5.0. The rationale behind this decision is the same as the one described here

Using the beta version caused an issue in which a node package was not properly installed when running npm i in the civihr root, thus causing gulp commands to not execute

I couldn't find the reason for it, so instead of using the beta version I used the latest stable version (v3.5.2). It doesn't contain the improvement that the beta contains, but it's at least stable

Fixed AngularJS urls

After #2790 , the format of urls of angular apps have changed, for example:

- /civicrm/tasksassignments/dashboard#/tasks
+ /civicrm/tasksassignments/dashboard#!/tasks

- /manager-leave#/manager-leave/requests
+ /manager-leave#!/manager-leave/requests

which broke BackstopJS scenarios where we used those urls directly

{
  "label": "SSP / My Leave / Report",
  "url": "{{siteUrl}}/my-leave#/my-leave/report"
}

This PR fixes the urls

Fixed selector for edit my details button

The worked done in as part of compucorp/civihr-employee-portal#520 changed the href value of the "Edit my details" button,

- [href="/my_details/nojs/view"]
+ [href="/edit-my-personal-details/js/view"]

which resulted in the "SSP / Edit My Details" as its script relied on the value of href, and didn't get updated. The PR fixes the issue by updating the selector

@AkA84 AkA84 force-pushed the PCHR-4098-fix-backstopjs-angular-apps-urls branch from 0f6ff40 to 639c60b Compare August 6, 2018 11:19
@AkA84 AkA84 force-pushed the PCHR-4098-fix-backstopjs-angular-apps-urls branch from 639c60b to ea6131c Compare August 7, 2018 14:51
@AkA84 AkA84 force-pushed the PCHR-4098-fix-backstopjs-angular-apps-urls branch from ea6131c to 4cdd46a Compare August 7, 2018 14:55
@AkA84 AkA84 changed the title PCHR-4098: Fix BackstopJS angular apps urls PCHR-4098: Misc BackstopJS fixes and improvements Aug 7, 2018
@igorpavlov-zz
Copy link
Contributor

Please remove the enhancement and add a bug label, since you say this PR "fixes" something.

@igorpavlov-zz
Copy link
Contributor

You update BS to 3.5.0, could you please mention that in the PR desc?

@igorpavlov-zz
Copy link
Contributor

igorpavlov-zz commented Aug 8, 2018

The PR description is misleading. You specify version 3.5.0 too prominently, in the header, so if someone skims the PR, they may miss the point that it is actually 3.5.2 version that is used. Thus I suggest the following desc structure:

[h1]Upgrade BackstopJS  to v3.5.2[h1]

Initially in this PR BackstopJS was updated to the Beta version (currently v3.5.0). The rationale behind this decision is the same as the one described [here]

However, using the beta version caused an issue in which a node package was not properly installed when running `npm i` in the civihr root, thus causing gulp commands to not execute

I couldn't find the reason for it, so instead of using the beta version I used the latest stable version (v3.5.2). It doesn't contain the improvement that the beta contains, but it's at least stable.

Copy link
Contributor

@igorpavlov-zz igorpavlov-zz left a comment

Choose a reason for hiding this comment

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

Please update the PR desc as suggested above or suggest our own version that addresses the issue described above.

@AkA84 AkA84 merged commit 39304a6 into staging Aug 8, 2018
@AkA84 AkA84 deleted the PCHR-4098-fix-backstopjs-angular-apps-urls branch August 8, 2018 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants