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

Routes and Controllers for the new Additional Information Page #34

Merged
merged 20 commits into from
Oct 11, 2024

Conversation

lexik04
Copy link

@lexik04 lexik04 commented Oct 9, 2024

Issue #24

*This sub-issue spreads across 2 branches: infoButton 4 and infoButton5

What and Why
This sub-issue is for creating the Additonal Information Page. The purpose of the sub issue is to have it lead to a working new endpoint page that has the additonal information page with a form. Here is the PR for the form on the front end repo: CMU-313/nodebb-frontend-f24-team-tbd-1#2

Current Status
It took the second half of sprint 1 and all of sprint 2 to get the new page to work and not show "page not found". The errror that caused this was on src/routes/user.js:

Original:

const additionalInfoController = require('../additional-info');// Generated by ChatGPT

Fix:

const additionalInfoController = require('../controllers/additional-info');// Generated by ChatGPT

The file was unable to found due to the incorrect location.

Screenshot 2024-10-10 at 7 54 45 PM

Process/Solution
For the routes files I worked on the api file, index file, user file, and admin file

Final look of what was added to src/routes/api file (code was generated by chatgpt)

router.get('/admin/additional-info', (req, res) => {
		const response = {
			info: 'This is the additional information.',
		};
		res.status(200).json(response);
	});

Final look of what was added to src/routes/user file

const additionalInfoController = require('../controllers/additional-info');// Generated by ChatGPT
setupPageRoute(app, '/additional-info', [], additionalInfoController.get); // Generated by ChatGPT

For the controller, I have created and worked on the index file and the newly created additional info file :

Final look of what was added to src/controllers/index file:

Controllers.init = async function (params) {
	const { router } = params;
	// Register the /additional-info route
	router.get('/additional-info', Controllers.additionalInfo.get);
};

Final look of src/controllers/additional-info file:

'use strict';

// generated by chatGPT
const additionalInfoController = module.exports;

additionalInfoController.get = async function (req, res) {
	// You can customize the data being sent to the template
	const data = {
		title: 'Additional Information',
	};

	// Render the additional-info template without the 'templates/' prefix
	res.render('additional-info', data);
};

Testing
Alhtough it hasn't worked yet, I wanted to write tests:

src/test/controllers:

{ it: 'should load additional information page', url: `/additional-info` },

src/tes/controllers-admin:

it('should load additional information page', async () => {
		const { response, body } = await request.get(`${nconf.get('url')}/admin/additional-info`, { jar: jar });
		assert.equal(response.statusCode, 200);
		assert(body);
	});

Front-end
All of the front-end logic is in this repo: https://github.com/CMU-313/nodebb-frontend-f24-team-tbd-1.git

The 2nd PR contains the code for the additional information button and the code for the form on the page.

Additional Suggestion
I would go through the code to see if there have been any changes that were unnecessary, especially in the routes to ensure code quality.

@lexik04 lexik04 requested review from ay0503 and KevnKey October 9, 2024 22:01
@lexik04 lexik04 self-assigned this Oct 9, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 11263827176

Details

  • 5 of 11 (45.45%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 82.655%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/controllers/additional-info.js 2 4 50.0%
src/controllers/index.js 2 4 50.0%
src/routes/index.js 1 3 33.33%
Totals Coverage Status
Change from base Build 11041313566: -0.01%
Covered Lines: 22327
Relevant Lines: 25593

💛 - Coveralls

@lexik04 lexik04 added bug Something isn't working enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Oct 10, 2024
Copy link

@ay0503 ay0503 left a comment

Choose a reason for hiding this comment

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

lgtm!

@@ -160,6 +160,7 @@ middleware.privateTagListing = helpers.try(async (req, res, next) => {
next();
});


Copy link

Choose a reason for hiding this comment

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

Nit: we can probably remove this empty line but everything else lgtm!

@lexik04 lexik04 removed bug Something isn't working help wanted Extra attention is needed question Further information is requested labels Oct 11, 2024
@lexik04 lexik04 merged commit 9aa76de into f24 Oct 11, 2024
1 check failed
@lexik04 lexik04 requested a review from vsolskyyy October 11, 2024 00:43
Copy link

@vsolskyyy vsolskyyy left a comment

Choose a reason for hiding this comment

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

Very well organized code, the comments are helpful.

@lexik04 lexik04 changed the title Routes and Controllers for the new Additonal Information Page Routes and Controllers for the new Additional Information Page Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants