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

Move disclosures to fetch-logic #8008

Merged
merged 2 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/functional-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,13 @@ jobs:
run: ./refresh-data.sh test.sql.gz

- name: Run Cypress
uses: cypress-io/github-action@v5
uses: cypress-io/github-action@v6.6.0
with:
build: yarn build
start: python cfgov/manage.py runserver 0.0.0.0:8000
wait-on: 'http://localhost:8000'
spec: test/cypress/integration/paying-for-college/disclosures/disclosures-basics.cy.js

env:
MAPBOX_ACCESS_TOKEN: ${{ secrets.MAPBOX_ACCESS_TOKEN }}
CYPRESS_ENVIRONMENT: github
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
{% endblock %}

{% block content %}
{% set school_primary_alias = school.primary_alias if school else '' %}
<main id="main" data-context="{{url_root}}">
<div id="financial-offer" class="understanding-financial-aid-offer">
<section class="m-hero">
Expand Down Expand Up @@ -83,7 +84,7 @@ <h2 class="verify_prompt">
</h2>
<form class="verify_form">
<div class="verify_school">
{{school.primary_alias}}
{{school_primary_alias}}
</div>
<div class="verify_location">
{% if school %}{{school.city}}, {{school.state}}{% endif %}
Expand Down Expand Up @@ -258,7 +259,7 @@ <h2 class="step_heading">
<p class="step_intro">
Here is your financial aid offer from
<span
id="intro__school-name">{{school.primary_alias}}</span>. Please review the amounts provided by
id="intro__school-name">{{school_primary_alias}}</span>. Please review the amounts provided by
your school below and make any necessary changes
or add any missing information. Please note any
changes you make here will not change your
Expand Down Expand Up @@ -886,7 +887,7 @@ <h4 class="aid-form_summary-heading">
<p>
This is an estimate of the remaining costs
of studying your program at
{{school.primary_alias}} for one year.
{{school_primary_alias}} for one year.
</p>
</div>
</div>
Expand Down Expand Up @@ -1742,7 +1743,7 @@ <h4 class="aid-form_summary-heading">
<p>
This is an estimate of the remaining costs
of studying your program at
{{school.primary_alias}} for one year based
{{school_primary_alias}} for one year based
on your inputs above.
</p>
</div>
Expand Down Expand Up @@ -1873,7 +1874,7 @@ <h4 class="aid-form_summary-heading">
After all the grants, scholarships, loans,
and personal contributions, this is how
much you still need to pay to attend
{{school.primary_alias}} for one year.
{{school_primary_alias}} for one year.
</p>
</div>
<div class="aid-form_summary debt-summary
Expand Down Expand Up @@ -2003,7 +2004,7 @@ <h3 class="criteria_heading">
<p class="content_grad-cohort">
For first-time students enrolled full-time
in {{program.program_name}} at
{{school.primary_alias}},
{{school_primary_alias}},
{{program.completers}} out of
{{program.completion_cohort}}
graduated.
Expand Down Expand Up @@ -2942,7 +2943,7 @@ <h2 class="step_heading step_settlement">
It's estimated you'll owe <span
data-financial="loanLifetime">[XX]</span>
to complete this program in {{program.program_name}} at
{{school.primary_alias}}. Do you better
{{school_primary_alias}}. Do you better
understand how this may impact your future
finances?
</h2>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,38 +1,20 @@
import { promiseRequest } from '../utils/promise-request.js';
import financialView from '../views/financial-view.js';

/**
* getApi - Make an API request to the endpoint specified with parameters specified
* @param {string} url - URL of API endpoint
* @returns {object} Promise
*/
function getApi(url) {
return new Promise(function (resolve, reject) {
promiseRequest('GET', url)
.then(function (resp) {
resolve(resp);
})
.catch(function (error) {
reject(new Error(error));
});
});
}

const getApiValues = {
values: {},
constants: function () {
const urlBase = document.querySelector('main').getAttribute('data-context');
const url =
'/' + urlBase + '/understanding-your-financial-aid-offer/api/constants/';
return getApi(url);
return fetch(url);
},

expenses: function () {
const urlBase = document.querySelector('main').getAttribute('data-context');
const url =
'/' + urlBase + '/understanding-your-financial-aid-offer/api/expenses/';

return getApi(url);
return fetch(url);
},

fetchSchoolData: function (iped) {
Expand All @@ -44,15 +26,9 @@ const getApiValues = {
iped +
'/';

return new Promise(function (resolve ) {
promiseRequest('GET', url)
.then(function (resp) {
resolve(resp);
})
.catch(function (error) {
financialView.missingData('school');
new Error(error);
});
return fetch(url).catch(function (error) {
financialView.missingData('noSchool');
return new Error(error);
});
},

Expand All @@ -77,26 +53,9 @@ const getApiValues = {
pid +
'/';

// $.ajax({
// url: url,
// dataType: 'json',
// success: function (resp) {
// return resp;
// },
// error: function (/* req, status, err */) {
// financialView.missingData('program');
// },
// });

return new Promise(function (resolve ) {
promiseRequest('GET', url)
.then(function (resp) {
resolve(resp);
})
.catch(function () {
financialView.missingData('school');
// reject(new Error(error));
});
return fetch(url).catch(function (error) {
financialView.missingData('noProgram');
return new Error(error);
});
},

Expand All @@ -114,14 +73,14 @@ const getApiValues = {
url += '_' + pid + '/';
}

return getApi(url);
return fetch(url);
},

schoolData: function (iped, pid) {
return Promise.all([
this.fetchSchoolData(iped),
this.fetchProgramData(iped, pid),
this.fetchNationalData(iped, pid),
this.fetchSchoolData(iped).then((v) => v.json()),
this.fetchProgramData(iped, pid).then((v) => v.json()),
this.fetchNationalData(iped, pid).then((v) => v.json()),
]);
},

Expand All @@ -130,16 +89,14 @@ const getApiValues = {
const warning = document
.querySelector('[data-warning]')
.getAttribute('data-warning');
if (warning !== '' && typeof warning !== 'undefined') {
if (warning && !window.Cypress) {
financialView.missingData(warning);
Copy link
Member

Choose a reason for hiding this comment

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

Should this code know about Cypress? It seems like whatever the need is here should be offloaded to the test logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this is kinda gross, the problem is though that the data isn't available in the test db, so this warning code will always trigger, even though the actual http requests are stubbed.

Putting this logic here allows Cypress to skip the warning code (which we want) and, in normal use of the app, allows the warning conditional to trigger when we need it.

I think maybe the best solution would be rearchitecting how the warning logic works/how the warning is propagated from the backend to the frontend, but that seemed out of scope.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'd be happy to fast-follow that because this is pretty gross. 😁

const deferred = {};
deferred.promise = new Promise((resolve, reject) => {
deferred.resolve = resolve;
deferred.reject = reject;
});
return deferred;
return Promise.resolve();
}
return Promise.all([this.constants(), this.expenses()]);
return Promise.all([
this.constants().then((v) => v.json()),
this.expenses().then((v) => v.json()),
]);
},
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import $ from './utils/dollar-sign.js';
import fetch from './dispatchers/get-api-values.js';
import getApiValues from './dispatchers/get-api-values.js';
import verifyOffer from './dispatchers/post-verify.js';
import financialModel from './models/financial-model.js';
import schoolModel from './models/school-model.js';
Expand Down Expand Up @@ -38,10 +38,9 @@ const ready = function (callback) {
const app = {
urlValues: {},
init: function () {
// jquery promise to delay full model creation until ajax resolves
fetch.initialData().then((resp) => {
const constants = JSON.parse(resp[0].responseText);
const expenses = JSON.parse(resp[1].responseText);
getApiValues.initialData().then((resp) => {
if (!resp) return;
const [constants, expenses] = resp;
financialModel.init(constants[0]);
financialView.init();
if (location.href.indexOf('about-this-tool') === -1) {
Expand All @@ -51,12 +50,10 @@ const app = {
if (getUrlOfferExists()) {
// Check for URL offer data
this.urlValues = getUrlValues();
fetch
getApiValues
.schoolData(this.urlValues.collegeID, this.urlValues.programID)
.then((respArr) => {
const schoolData = JSON.parse(respArr[0].responseText);
const programData = JSON.parse(respArr[1].responseText);
const nationalData = JSON.parse(respArr[1].responseText);
const [schoolData, programData, nationalData] = respArr;
const data = {};
Object.assign(data, schoolData, programData, nationalData);
const schoolValues = schoolModel.init(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**

Check warning on line 1 in cfgov/unprocessed/apps/paying-for-college/js/disclosures/utils/dollar-sign.js

View workflow job for this annotation

GitHub Actions / frontend

Missing JSDoc @returns declaration
*
* @param selector

Check warning on line 3 in cfgov/unprocessed/apps/paying-for-college/js/disclosures/utils/dollar-sign.js

View workflow job for this annotation

GitHub Actions / frontend

Missing JSDoc @param "selector" description

Check warning on line 3 in cfgov/unprocessed/apps/paying-for-college/js/disclosures/utils/dollar-sign.js

View workflow job for this annotation

GitHub Actions / frontend

Missing JSDoc @param "selector" type
*/
function Query(selector) {
this.elements = [];
Expand All @@ -17,17 +17,11 @@
if (typeof selector === 'string' && selector !== '') {
this.elements = document.querySelectorAll(selector);
}

// if ( this.elements.length > 1 ) {
// return this.elements;
// } else {
// return this.elements[0];
// }
}

Query.prototype.attr = function (name, value) {
if (typeof value === 'undefined') {
return this.elements[0].getAttribute(name);
return this.elements.length ? this.elements[0].getAttribute(name) : null;
} else {
this.elements.forEach((elem) => {
elem.setAttribute(name, value);
Expand All @@ -36,13 +30,13 @@
};

Query.prototype.cloner = function () {
return this.elements[0].cloneNode(true);
return this.elements.length ? this.elements[0].cloneNode(true) : null;
};

Query.prototype.text = function (value) {
// getter
if (typeof value === 'undefined') {
return this.elements[0].textContent;
return this.elements.length ? this.elements[0].textContent : null;
}
//setter
else {
Expand All @@ -55,7 +49,7 @@
Query.prototype.val = function (value) {
// getter
if (typeof value === 'undefined' && this.elements.length > 0) {
return this.elements[0].value;
return this.elements.length ? this.elements[0].value : null;
}
//setter
else {
Expand Down Expand Up @@ -146,19 +140,11 @@
};

Query.prototype.height = function () {
if (this.elements.length > 0) {
return this.elements[0].offsetHeight;
} else {
return null;
}
return this.elements.length ? this.elements[0].offsetHeight : null;
};

Query.prototype.top = function () {
if (this.elements.length > 0) {
return this.elements[0].offsetTop;
} else {
return null;
}
return this.elements.length ? this.elements[0].offsetTop : null;
};

Query.prototype.listen = function (eventType, callback) {
Expand All @@ -168,7 +154,7 @@
};

Query.prototype.tagName = function () {
return this.elements[0].tagName;
return this.elements.length ? this.elements[0].tagName : null;
};

Query.prototype.addClass = function (classNames) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,11 @@ const financialView = {
*/
updateViewWithProgram: function (values, urlValues) {
// Update program length
const lengthExists = Object.prototype.hasOwnProperty.call(urlValues, 'urlProgramLength');
if ( lengthExists ) {
const lengthExists = Object.prototype.hasOwnProperty.call(
urlValues,
'urlProgramLength',
);
if (lengthExists) {
this.$programLength.val(urlValues.urlProgramLength / 12).change();
} else {
this.$programLength.val(values.programLength).change();
Expand Down Expand Up @@ -959,18 +962,14 @@ const financialView = {
*/
missingData: function (dataType) {
$('.verify_wrapper').hide();
if (
$('[data-missing-data-error]:not([style*="display: none"]').length === 0
) {
$('[data-missing-data-error="' + dataType + '"]').show();
}
$('[data-missing-data-error="' + dataType + '"]').show();
},

/**
* Listener function for change events on financial INPUT fields
*/
financialInputChangeListener: function () {
$('[nancial]').each((elmo) => {
$('[data-financial]').each((elmo) => {
elmo.addEventListener('change', (event) => {
const dataFinancial = event.target.dataset.financial;
if (dataFinancial) {
Expand Down
Loading