Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

[User accounts] Add project option to user accounts #5009

Merged
merged 12 commits into from
Oct 30, 2019
Merged

[User accounts] Add project option to user accounts #5009

merged 12 commits into from
Oct 30, 2019

Conversation

ridz1208
Copy link
Collaborator

@ridz1208 ridz1208 commented Aug 7, 2019 •
edited by xlecours
Loading

Brief summary of changes

Add the functionality to affiliate users to specific projects.

At this stage affiliation with a project does not affect any of the data access, that will be added later on.

Changes in this PR were made to very closely resemble the CenterID/User interaction for consistency

Testing instructions (if applicable)

  1. Try creating new users with projects
  2. try adding/removing projects from existing users

This PR depends on the changes in #4919 for the changes in the Project table SQL. Here is the diff ridz1208/Loris@2019_07_01_project_to_registration_project...ridz1208:2019-08_05_add_user_project

@ridz1208 ridz1208 added Feature PR or issue introducing/requiring at least one new feature Blocked PR awaiting the merge, resolution or rejection of an other Pull Request labels Aug 7, 2019
@ridz1208 ridz1208 force-pushed the 2019-08_05_add_user_project branch 4 times, most recently from e0d5dd6 to e532c26 Compare August 8, 2019 00:53
ridz1208
ridz1208 commented Aug 8, 2019
View reviewed changes
test/unittests/CandidateTest.php Outdated
@@ -298,8 +298,8 @@ public function testGetProjectTitle()
$this->_candidate->select($this->_candidateInfo['CandID']);

$this->_dbMock->expects($this->any())
->method('pselect')
->willReturn($this->_listOfProjects);
->method('pselectColWithIndexKey')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@AlexandraLivadas @driusan had to chnage this one for it to pass travis

@driusan
Copy link
Collaborator

driusan commented Sep 13, 2019

@ridz1208 Why is this marked as blocked?

@ridz1208
Copy link
Collaborator Author

Depends on 4919. See description

@ridz1208 ridz1208 added Needs Rebase PR contains conflicts with the current target branch or was issued to the wrong branch and removed Blocked PR awaiting the merge, resolution or rejection of an other Pull Request labels Sep 19, 2019
@ridz1208 ridz1208 removed the Needs Rebase PR contains conflicts with the current target branch or was issued to the wrong branch label Sep 20, 2019
@cmadjar cmadjar assigned Jkat and unassigned kchatpar Oct 8, 2019
@cmadjar
Copy link
Collaborator

cmadjar commented Oct 8, 2019

@Jkat Can you please review this PR before Thursday? Thank you!!

Jkat
Jkat suggested changes Oct 9, 2019
View reviewed changes
modules/user_accounts/php/edit_user.class.inc Outdated
Comment on lines 731 to 734
$siteOptions = array();
$site = array();

$siteOptions = array();
Copy link
Contributor

Choose a reason for hiding this comment

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

y

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i dont know

modules/user_accounts/php/edit_user.class.inc Outdated
//======================================
// Ensure that at least one site is selected
if (empty($values['ProjectIDs'])) {
$errors['projectss_group'] = "You must select at least one " .
Copy link
Contributor

Choose a reason for hiding this comment

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

projects_group

@Jkat
Copy link
Contributor

Jkat commented Oct 9, 2019

Code changes look fine, my sandbox is still blocked because of database administration issues, unable to test frontend at the moment.

@Jkat
Copy link
Contributor

Jkat commented Oct 10, 2019

I just tested it and it seems to work fine. My sandbox is kinda in a screwed up state though. But yeah should be just those 2 small fixes in my review.

@ridz1208
Copy link
Collaborator Author

@Jkat fixed !!

@driusan
Copy link
Collaborator

driusan commented Oct 29, 2019

This is going to fail Travis when it finishes

FILE: /home/travis/build/aces/Loris/php/libraries/User.class.inc

----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 307 | ERROR | [x] Whitespace found at end of line
 419 | ERROR | [x] Whitespace found at end of line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

@ridz1208 ridz1208 added the Blocking PR should be prioritized because it is blocking the progress of another task label Oct 29, 2019
@ridz1208 ridz1208 mentioned this pull request Oct 29, 2019
Merged
@driusan driusan merged commit 6bf85ce into aces:major Oct 30, 2019
@ridz1208 ridz1208 added this to the 22.0.0 milestone Oct 30, 2019
@christinerogers
Copy link
Contributor

This PR was based on
#4255
with work by Zain and especially Jacob Penny from the CAP project, and initially committed for Loris by Justin.

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Jkat Jkat Jkat requested changes

Assignees

@Jkat Jkat

Labels
Blocking PR should be prioritized because it is blocking the progress of another task Feature PR or issue introducing/requiring at least one new feature
Projects
None yet
Milestone
22.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@ridz1208 @driusan @cmadjar @Jkat @christinerogers @kchatpar