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

Migrate intelv2 aa1 to implement 2 #132

Open
wants to merge 192 commits into
base: implement-2
Choose a base branch
from
Open

Conversation

pinterid
Copy link
Member

Checklist:

  • Have you added an explanation of what your changes do and why you'd like them to be included?
  • Have you updated or added documentation for the change?
  • Have you tested your changes with successful results?

Type of Changes:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation (non-breaking change which adds documentation)
  • Breaking change (fix or feature that would cause existing functionality to change)

What is the current behavior? (link to any open issues here)

What is the new behavior (if this is a feature change)?

Other information:

Aichnerc and others added 30 commits September 7, 2020 16:00
Yeet old out of existence, yeet new into existence.
The register component has been reworked.
The labels have been improved.
All actions have been removed.
User actions like login, logout, and getPerson
have been implemented.
LoginForm and HomePage have been adjusted to
work with the new user actions and state definition.
Code was commented out to avoid errors caused by
deleting the actions.
Now it is possible to fetch persons which get stored in the store
as fetchedPerson.
The logout action has been imported from another
location.
Cleanup and person action integration
Incorrect dispatches in the person action have
been fixed.
Some non existent action has been removed.
The modal to connect other accounts has been added.
Some functions have been introduced.
Now it is possible again to search users.
A action for getting all gitlab servers for registration
has been added.
Some more functionality has been added.
Types have been improved.
Naming has been improved.
The get gitlab servers action is now exported correctly.
Also undefined variables have been removed.
The reducers are now correctly added to the root
reducer.
The registration has been unyeeted.
The registration action has been added.
It calls the login action after.
Get persons brief and get person have been moved
to the general actions.
The person actions should only contain actions for the
currently logged person.
Following actions and their reducers have been added:
- All Achievements
- Person Settings
- Person Meta Link
- Person Profiles
- Person Add Profile
- Person Delete Profile
- Person Update Profile
- Person Instagram Posts
- Person Process Profiles
- Person Follow
- Person Unfollow
- Person Like
- Person Unlike
Following actions and their reducers have been added:
- Enterprise General
- Enterprise Projects
- Enterprise Users
Following actions and their reducers have been added:
- Redeem Achievement
A type has been fixed. Yey.
The export of register in the user action
has been added.
Now it is possible to check if a specific
username is already taken.
pinterid and others added 14 commits September 14, 2020 10:25
The photo map data and display was updated.
Now the map is only displayed if displayMap is enabled.
it is no checked if the user is logged in and if its his profile.
The modal should now only show up at the own page and now close without errors.
Now the talks page works again. 
It is also possible to leave a comment.
The action now returns an achievement.
The achievement tab was updated.
Now the select images button is only displayed if at least
one instagram profile is connected.
Now intel and client no longer uses a local version of the snek
engine but the production one on engine.snek.at.
Some dependencies have been updated due to security
vulnerabilities and intel and client releases.
Copy link
Member

@schettn schettn left a comment

Choose a reason for hiding this comment

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

Have a look

src/Routes.js Outdated
Comment on lines 36 to 40
<Route
exact
path="/t/:username/:uid"
component={(props) => <TalkPage {...props} />}
path="/temp"
component={(props) => <TempPage {...props} />}
/>
Copy link
Member

Choose a reason for hiding this comment

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

All "Temp" component should be removed.

calculateSources = (nextPropsYear) => {
const { statistic } = this.props;
calculateSources = (nextPropsYearIndex) => {
const { yearsStatistic, currentStatistic } = this.props;

let totalReviews = 0;
let totalIssues = 0;
let totalRequests = 0;
let totalCommits = 0;
let totalSources = 1;
Copy link
Member

Choose a reason for hiding this comment

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

@Aichnerc why is totalSources = 1. Shouldn't it be equals to 0?

Comment on lines 26 to 28
state = {
loggedUser: null,
};
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better not to store the whole loggedUser into the state.
We should load only required datasets into the state.

Comment on lines 51 to 62
let loggedUser = this.props.loggedUser;
let follows = [];

for (let count in loggedUser.person.follows) {
follows.push(loggedUser.person.follows[count]);
}

follows.push({ slug: "p-" + personToFollow });

loggedUser.person.follows = follows;

this.setState({ loggedUser });
Copy link
Member

Choose a reason for hiding this comment

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

This should be reworked.

  • Use the new state defined above.
  • Then remove the for loop. This workaround is no longer required. (loggedUser.follows is readOnly)

Comment on lines 74 to 80
for (let count in loggedUser?.person?.follows) {
if (loggedUser.person.follows[count].slug !== "p-" + personToUnfollow) {
follows.push(loggedUser.person.follows[count]);
}
}

loggedUser.person.follows = follows;
Copy link
Member

Choose a reason for hiding this comment

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

This should be reworked:

  • Use the new state defined above
  • Rework the remove algorithm as follows:
   var members = [
        {name: 'Anna', class: 'one'}, 
        {name: 'Bob', class: 'two'},  
        {name: 'Chuck', class: 'two'}];

   var idx = members.findIndex(p => p.class=="two");
   var removed = members.splice(idx,1);
     
   console.log(removed);
   console.log(members);

Comment on lines 89 to 95
for (let count in loggedUser?.person?.follows) {
if (loggedUser.person.follows[count].slug === follower.slug) {
return true;
}
}

return false;
Copy link
Member

Choose a reason for hiding this comment

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

Rework with arr.find()

Comment on lines +25 to +36
class LikesModal extends React.Component {
state = {
loggedUser: null,
};

componentDidMount = () => {
const { loggedUser } = this.props;

this.setState({
loggedUser: loggedUser,
});
};
Copy link
Member

Choose a reason for hiding this comment

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

Rework like FollowModal.

Comment on lines +67 to +69
for (let count in this.state.achievements) {
achievements.push(this.state.achievements[count]);
}
Copy link
Member

Choose a reason for hiding this comment

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

Why?

pinterid and others added 8 commits November 10, 2020 10:58
The code quality was improved due to a request by Codacy.
The temp page which implemented all newer
components is removed now because it is no longer
required.
This code was never used therefore I removed it.
The new implementation is more suitable for larger ones
records but does not fully work yet
The page refreshing has been improved.
Now every time handleModalClose is called,
the infocard gets new data.
All uncommented console logs have been removed.
Copy link
Member

@kleberbaum kleberbaum left a comment

Choose a reason for hiding this comment

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

Should be ok to merge.

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.

4 participants