update edit page 1396 #1401

plang-psm · 2023-06-20T05:36:45Z

What changes did you make and why did you make them ?

Updated the styling to match the Figma document.
Updated the PATCH request to accept all changes of the form instead of individual changes.
** Radio buttons do not save -- see below

Issue:

The location radio buttons do not have a field in the DB causing us to lose the value when transferring data to the edit page.

My suggestion:

Add another field to the projects collection in the DB so we can set/update the value and pass it through to the radio buttons. This would need to be done in the Add Project Form before adding to the Edit Form.

Extra

I left some state/change solution that could be used if needed when the time comes. Also, we should explore making a reusable Form component that takes in an array of inputs to keep @MattPereira 's great styling!

Visuals before changes are applied

Visuals after changes are applied

github-actions · 2023-06-20T05:37:19Z

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b plang-psm-edit-page-update-1396 development
git pull https://github.com/plang-psm/VRMS.git edit-page-update-1396

bkmorgan3

@plang-psm I left a comment down below but marking this reviewed, officially.

bkmorgan3 · 2023-06-20T23:57:58Z

client/src/components/manageProjects/editProject.js

+        <Divider sx={{ borderColor: 'rgba(0,0,0,1)' }} />
+        <Box sx={{ py: 2, px: 4 }}>
+          <form id="project-form" onSubmit={handleSubmit}>
+            {simpleInputs.map((input, event) => (


@plang-psm
1st - Good job this works as expected
2- Not sure if this is our decision maybe we can talk this out with design. I think there should be an edit button to make the fields editable so a user knows they are in Edit Mode.
3- Can any Admin user edit any existing project?

Also would the Patch request be a Put since the entire Project Object is being submitted?

Good catch @bkmorgan3 . You are correct, it should be a PUT instead of a PATCH because I am submitting all the data. I went ahead and updated it.

In regards to 2, I just went with whatever was on the Figma design but I know that is a feature we did talk about once. Wasn't sure if it was decided on so I just applied the UI.

In regards to 3, I am not completely sure. From my understanding only admin and project managers on that specific team (?) are able to view the projects and edit them. I don't think users or PMs not on that team have access to them.

Yeah, this is good work.

I agree we should look for an indication that the project is being edited. I'll ping the design team about that.

@juliagab56 Can you wiegh in on this?

Seems like it should go here - there's a spot for it where the Add Project SVG is:

Add Project

Edit Project

Disregard all the little letters in the boxes, that's an extension of mine

Spiteless

This is well on its way!

Stoked to see this coming together, thanks for getting the database update right!

@bkmorgan3 has an open PR as well that's adding in form validation for the AddProject component, and I think we'll end up pulling some of his work into this, but I think once we get rid of the additional lint changes this can be merged in!

Spiteless · 2023-07-01T06:57:14Z

backend/routers/projects.router.test.js

@@ -73,7 +64,7 @@ describe('UPDATE', () => {

    // Update project
    const res2 = await request
-      .patch(`/api/projects/${res.body._id}`)
+      .put(`/api/projects/${res.body._id}`)


I believe you've only materially edited one line in this file. It's hard to pick out what's being worked on and what isn't if we get lots of changes from your linter. Can you please go and pick out the specific part of the code you're changing? These steps shoud do the trick:

Rebase recent changes

git rebase -i HEAD~4

Locate commit with your code

pick 208f364 update edit page 1396 pick d33c125 update to PUT request pick b11873a change patch to put in test // This one pick 159309a adding patch back to router due to test failing for delete

Change pick to edit on b11873a -- this brings your repo to that specific state so you can change / edit things

pick 208f364 update edit page 1396 pick d33c125 update to PUT request edit b11873a change patch to put in test // This one pick 159309a adding patch back to router due to test failing for delete

Save and close file

Git now brings you to that specific commit state. Reset your changes to the previous HEAD. This unstages the changes you've made in this specific commit.

git reset HEAD^

Now use git add --patch to select the specific part of your edit that is relevent to this commit:

git add --patch backend/routers/projects.router.test.js

This will bring up an interactive selection in your terminal (complete with colors that won't show in markdown)

// Submit a project - const res = await request - .post('/api/projects/') - .set(headers) - .send(submittedData); + const res = await request.post('/api/projects/').set(headers).send(submittedData); expect(res.status).toBe(201); done(); }); (1/5) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?

What do these mean? [y,n,q,a,d,j,J,g,/,e,?]

patch This lets you choose one path out of a status like selection. After choosing the path, it presents the diff between the index and the working tree file and asks you if you want to stage the change of each hunk. You can select one of the following options and type return: y - stage this hunk n - do not stage this hunk q - quit; do not stage this hunk nor any of the remaining ones a - stage this hunk and all later hunks in the file d - do not stage this hunk nor any of the later hunks in the file g - select a hunk to go to / - search for a hunk matching the given regex j - leave this hunk undecided, see next undecided hunk J - leave this hunk undecided, see next hunk k - leave this hunk undecided, see previous undecided hunk K - leave this hunk undecided, see previous hunk s - split the current hunk into smaller hunks e - manually edit the current hunk ? - print help

Sift through the changes by hunt to get the right one (n - no, pass. y, yes, keep)

... (1/5) Stage this hunk [y,n,q,a,d,K,j,J,g,/,e,?]? n (2/5) Stage this hunk [y,n,q,a,d,K,j,J,g,/,e,?]? n (3/5) Stage this hunk [y,n,q,a,d,K,j,J,g,/,e,?]? n ... // Update project const res2 = await request - .patch(`/api/projects/${res.body._id}`) + .put(`/api/projects/${res.body._id}`) .set(headers) .send(updatedDataPayload); expect(res2.status).toBe(200); (4/5) Stage this hunk [y,n,q,a,d,K,j,J,g,/,e,?]? y // This one! (5/5) Stage this hunk [y,n,q,a,d,K,j,J,g,/,e,?]? n

git status should now show a partial staged modification

git status MM backend/routers/projects.router.test.js ~ <-- This one is Green ~ <-- This one is Red

Commit these changes with amend

git commit --amend --no-edit

Finish out the rebase and force push up, overwriting your old commit with the linter changes.

git rebase --continue git push --force

I'll go ahead and fix the linting but I am leaning on scrapping most of this PR since we need to fix the Project Form. We currently need to separate the ProjectFrom component, AddProjectForm, and EditProjectForm component so we can reuse the form component for both the add and edit. It would be much cleaner and remove some unnecessary lines.

Spiteless · 2023-07-01T07:02:42Z

client/src/api/ProjectApiService.js

-      return projectDetails._id
+      const proj = await fetch(this.baseProjectUrl, requestOptions);
+      const projectDetails = await proj.json();
+      return projectDetails._id;


I hate to be a stickler about this stuff, but can you drop these changes? It makes it a lot harder to follow what is being changed the PR with a lot of other linting changes.

I know it's a hassle and we don't have a linter up and running for the repo yet, but adding the specifc section you're wanting to edit wth git add --patch ${file} or git add -p ${file} and picking the lines of your PR that aren't linting would make it a lot easier to process this.

There's a writeup in another comment on this PR review that has a walkthrough

Spiteless · 2023-07-01T07:05:13Z

client/src/api/ProjectApiService.js

    // Update database
    const url = `${this.baseProjectUrl}${projectId}`;
    const requestOptions = {
      method: 'PATCH',
      headers: this.headers,
-      body: JSON.stringify({ [fieldName]: updateValue }),
+      body: JSON.stringify({ ...projectData }),


If I'm understand this correclty, this change is because we're now submitting a whole JSON object to update the database entry intead of one specific field value. Nice work!

Spiteless · 2023-07-01T07:24:36Z

client/src/components/manageProjects/editProject.js

+        <Divider sx={{ borderColor: 'rgba(0,0,0,1)' }} />
+        <Box sx={{ py: 2, px: 4 }}>
+          <form id="project-form" onSubmit={handleSubmit}>
+            {simpleInputs.map((input, event) => (


Yeah, this is good work.

I agree we should look for an indication that the project is being edited. I'll ping the design team about that.

@juliagab56 Can you wiegh in on this?

Seems like it should go here - there's a spot for it where the Add Project SVG is:

Add Project

Edit Project

Disregard all the little letters in the boxes, that's an extension of mine

Spiteless · 2023-07-01T07:26:21Z

backend/routers/projects.router.js

@@ -1,4 +1,4 @@
-const express = require("express");
+const express = require('express');


Can you please drop this change as per other comments in this PR review re: linting

Spiteless · 2023-07-01T07:34:45Z

client/src/sass/ManageProjects.scss

@@ -313,7 +312,7 @@ textarea {
 }

 .event-list-details {
- display: flex;
+  display: flex;


Can you please drop this change as per other comments in this PR review re: linting

Easiest way to distinguish rn between editable content vs only view would be to make use of the form field when editing and have no element around the information when only viewed
ie:

You'd have to disregard the "[edit]" buttons. Not sure what the feedback on that was but we can revisit the idea of only having one button for editing instead of one for each field. I can have some updated designs for you all later this afternoon

Spiteless · 2023-07-01T08:01:42Z

@plang-psm

re:

Issue:

The location radio buttons do not have a field in the DB causing us to lose the value when transferring data to the edit page.

Here's the model of the location as we have it now:

const projectSchema = mongoose.Schema({
    name: { type: String },
    description: { type: String },
    githubIdentifier: { type: String },
    projectStatus: { type: String },                    // Active, Completed, or Paused

    location: { type: String },                         // DTLA, Westside, South LA, or Remote (hacknight)

    //teamMembers: { type: String },                    // commented since we should be able to get this from Project Team Members table
    createdDate: { type: Date, default: Date.now },     // date/time project was created
    completedDate: { type: Date },                      // only if Status = Completed, date/time completed
    githubUrl: { type: String },                        // link to main repo
    slackUrl: { type: String },                         // link to Slack channel
    googleDriveUrl: { type: String },
    googleDriveId: { type: String },
    hflaWebsiteUrl: { type: String },
    videoConferenceLink: { type: String },
    lookingDescription: { type: String },               // narrative on what the project is looking for
    recruitingCategories: [{ type: String }],           // same as global Skills picklist
    partners: [{ type: String }],                       // any third-party partners on the project, e.g. City of LA
    managedByUsers: [{ type: String }]                  // Which users may manage this project.                 
});

What do you think about infering whether it's in person or remote based on whether the value passes some test to confirm that it's a valid web URL?

FreeCodeCamp Article

const isValidUrl = urlString=> {
      try { 
      	return Boolean(new URL(urlString)); 
      }
      catch(e){ 
      	return false; 
      }
  }

If it passes that test, start off the radio button with remote, otherwise indicate in-person?

This way we wouldn't have to make changes to the db given that the plan is to move to another backend eventually.

juliagab56 · 2023-07-04T02:58:15Z

Figma file

plang-psm · 2023-07-10T23:01:20Z

@plang-psm

re:

Issue:

The location radio buttons do not have a field in the DB causing us to lose the value when transferring data to the edit page.

Here's the model of the location as we have it now:

const projectSchema = mongoose.Schema({
    name: { type: String },
    description: { type: String },
    githubIdentifier: { type: String },
    projectStatus: { type: String },                    // Active, Completed, or Paused

    location: { type: String },                         // DTLA, Westside, South LA, or Remote (hacknight)

    //teamMembers: { type: String },                    // commented since we should be able to get this from Project Team Members table
    createdDate: { type: Date, default: Date.now },     // date/time project was created
    completedDate: { type: Date },                      // only if Status = Completed, date/time completed
    githubUrl: { type: String },                        // link to main repo
    slackUrl: { type: String },                         // link to Slack channel
    googleDriveUrl: { type: String },
    googleDriveId: { type: String },
    hflaWebsiteUrl: { type: String },
    videoConferenceLink: { type: String },
    lookingDescription: { type: String },               // narrative on what the project is looking for
    recruitingCategories: [{ type: String }],           // same as global Skills picklist
    partners: [{ type: String }],                       // any third-party partners on the project, e.g. City of LA
    managedByUsers: [{ type: String }]                  // Which users may manage this project.                 
});

What do you think about infering whether it's in person or remote based on whether the value passes some test to confirm that it's a valid web URL?

FreeCodeCamp Article

const isValidUrl = urlString=> {
      try { 
      	return Boolean(new URL(urlString)); 
      }
      catch(e){ 
      	return false; 
      }
  }

If it passes that test, start off the radio button with remote, otherwise indicate in-person?

This way we wouldn't have to make changes to the db given that the plan is to move to another backend eventually.

I like the idea but testing to set a radio button every time you load a page may hinder the site's performance. We may be able to use mongoDBs .updateMany to set another field in our project objects in our DB and just set it as false? My only concern would be that all the projects would still need to be updated since the default would be false. We could have PMs go in an update that field or I am open to other ideas.

plang-psm · 2023-07-10T23:15:31Z

Love the design! My only concern would be that we would need to use one of the two styles if we want to have a reusable project form component. An alternative may be to grey out the fields when the edit button is disabled and removing it when in "edit mode". What do you think @juliagab56 @Spiteless

juliagab56 · 2023-07-28T00:32:54Z

@plang-psm I'm confused as to what's going on since I keep having different discussions on other issues about similar things. But in this case, it's personally fine with me if you want to make these small design decisions that will move the issue forward. The greyed out fields sounds like a good idea to me as a quick solution. I am working on having a latest set of wireframes with the most updated screens but I need to be able to align with devs to understand what stage we are at to do so. I am also missing some reqs and specs I'm gathering.

jbubar · 2023-07-28T16:15:36Z

This looks good to me. We will bring it up in the next monday meeting and Ill try to get it merged. Thanks for all your work @plang-psm

Spiteless · 2023-08-01T02:30:14Z

Talk on this PR from todays meeting:

@jbubar

Recurring events are the most important field, providing users that information first would be better
Save button is currently below the recurring events, might seem confusing to the user
Editing and updating recurring
This PR improves look and feel regardless

Moving forward, we should

Merge this PR
Create separate PR to move where the save buttons are

@Spiteless

This PR still needs indication to the user that the form is in an edit state
Make a new issue to adapt this design to accept the disabled prop, allow fields to become editable on click of disabled field, indicate to the user that the form is in an edit state via edit badge to the direct right of the Project Information h2

plang-psm · 2023-08-22T22:54:13Z

see #1471

update edit page 1396

208f364

plang-psm requested review from MicahBear, MattPereira and Spiteless June 20, 2023 05:37

MattPereira requested a review from bkmorgan3 June 20, 2023 16:03

bkmorgan3 reviewed Jun 21, 2023

View reviewed changes

plang-psm added 3 commits June 21, 2023 13:23

update to PUT request

d33c125

change patch to put in test

b11873a

adding patch back to router due to test failing for delete

159309a

plang-psm requested a review from bkmorgan3 June 21, 2023 20:41

MattPereira requested a review from vanessavun June 25, 2023 17:08

Spiteless reviewed Jul 1, 2023

View reviewed changes

juliagab56 mentioned this pull request Jul 3, 2023

Project Details screens (Create, Edit, View) #1408

Open

9 tasks

jbubar mentioned this pull request Jul 28, 2023

VRMS: ALL: Monday Meeting Agenda 📝 #1280

Open

Spiteless mentioned this pull request Aug 1, 2023

feat: add validation to editableField #1425

Merged

plang-psm closed this Aug 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update edit page 1396 #1401

update edit page 1396 #1401

plang-psm commented Jun 20, 2023

github-actions bot commented Jun 20, 2023

bkmorgan3 left a comment

bkmorgan3 Jun 20, 2023

bkmorgan3 Jun 21, 2023

plang-psm Jun 21, 2023 •

edited

Loading

Spiteless Jul 1, 2023 •

edited

Loading

Spiteless left a comment

Spiteless Jul 1, 2023 •

edited

Loading

plang-psm Jul 10, 2023

Spiteless Jul 1, 2023

Spiteless Jul 1, 2023 •

edited

Loading

Spiteless Jul 1, 2023 •

edited

Loading

Spiteless Jul 1, 2023

Spiteless Jul 1, 2023

juliagab56 Jul 3, 2023

Spiteless commented Jul 1, 2023 •

edited

Loading

Issue:

juliagab56 commented Jul 4, 2023

plang-psm commented Jul 10, 2023

Issue:

FreeCodeCamp Article

plang-psm commented Jul 10, 2023

juliagab56 commented Jul 28, 2023

jbubar commented Jul 28, 2023

Spiteless commented Aug 1, 2023

plang-psm commented Aug 22, 2023

		@@ -1,4 +1,4 @@
		const express = require("express");
		const express = require('express');

update edit page 1396 #1401

update edit page 1396 #1401

Conversation

plang-psm commented Jun 20, 2023

What changes did you make and why did you make them ?

Issue:

My suggestion:

Extra

github-actions bot commented Jun 20, 2023

bkmorgan3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plang-psm Jun 21, 2023 • edited Loading

Choose a reason for hiding this comment

Spiteless Jul 1, 2023 • edited Loading

Choose a reason for hiding this comment

Add Project

Edit Project

Spiteless left a comment

Choose a reason for hiding this comment

Spiteless Jul 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spiteless Jul 1, 2023 • edited Loading

Choose a reason for hiding this comment

Spiteless Jul 1, 2023 • edited Loading

Choose a reason for hiding this comment

Add Project

Edit Project

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spiteless commented Jul 1, 2023 • edited Loading

Issue:

juliagab56 commented Jul 4, 2023

plang-psm commented Jul 10, 2023

Issue:

plang-psm commented Jul 10, 2023

juliagab56 commented Jul 28, 2023

jbubar commented Jul 28, 2023

Spiteless commented Aug 1, 2023

plang-psm commented Aug 22, 2023

plang-psm Jun 21, 2023 •

edited

Loading

Spiteless Jul 1, 2023 •

edited

Loading

Spiteless Jul 1, 2023 •

edited

Loading

Spiteless Jul 1, 2023 •

edited

Loading

Spiteless Jul 1, 2023 •

edited

Loading

Spiteless commented Jul 1, 2023 •

edited

Loading