From 520b5a4f9067eebb6979465ecb4b009a8cd296b4 Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Thu, 9 May 2019 15:25:47 +0200 Subject: [PATCH] removed unnecessary confusing modify and delete actions --- scm-ui/src/config/modules/roles.js | 196 ++---------------------- scm-ui/src/config/modules/roles.test.js | 187 +--------------------- 2 files changed, 22 insertions(+), 361 deletions(-) diff --git a/scm-ui/src/config/modules/roles.js b/scm-ui/src/config/modules/roles.js index 7ea871ff09..5fd96f2105 100644 --- a/scm-ui/src/config/modules/roles.js +++ b/scm-ui/src/config/modules/roles.js @@ -22,17 +22,6 @@ export const CREATE_ROLE_SUCCESS = `${CREATE_ROLE}_${types.SUCCESS_SUFFIX}`; export const CREATE_ROLE_FAILURE = `${CREATE_ROLE}_${types.FAILURE_SUFFIX}`; export const CREATE_ROLE_RESET = `${CREATE_ROLE}_${types.RESET_SUFFIX}`; -export const MODIFY_ROLE = "scm/roles/MODIFY_ROLE"; -export const MODIFY_ROLE_PENDING = `${MODIFY_ROLE}_${types.PENDING_SUFFIX}`; -export const MODIFY_ROLE_SUCCESS = `${MODIFY_ROLE}_${types.SUCCESS_SUFFIX}`; -export const MODIFY_ROLE_FAILURE = `${MODIFY_ROLE}_${types.FAILURE_SUFFIX}`; -export const MODIFY_ROLE_RESET = `${MODIFY_ROLE}_${types.RESET_SUFFIX}`; - -export const DELETE_ROLE = "scm/roles/DELETE_ROLE"; -export const DELETE_ROLE_PENDING = `${DELETE_ROLE}_${types.PENDING_SUFFIX}`; -export const DELETE_ROLE_SUCCESS = `${DELETE_ROLE}_${types.SUCCESS_SUFFIX}`; -export const DELETE_ROLE_FAILURE = `${DELETE_ROLE}_${types.FAILURE_SUFFIX}`; - const CONTENT_TYPE_ROLE = "application/vnd.scmm-repositoryRole+json;v=2"; // fetch roles @@ -184,104 +173,24 @@ export function createRole(link: string, role: Role, callback?: () => void) { }; } -// modify role -export function modifyRolePending(role: Role): Action { - return { - type: MODIFY_ROLE_PENDING, - payload: role, - itemId: role.name - }; -} - -export function modifyRoleSuccess(role: Role): Action { - return { - type: MODIFY_ROLE_SUCCESS, - payload: role, - itemId: role.name - }; -} - -export function modifyRoleFailure(role: Role, error: Error): Action { - return { - type: MODIFY_ROLE_FAILURE, - payload: { - error, - role - }, - itemId: role.name - }; -} - -export function modifyRoleReset(role: Role): Action { - return { - type: MODIFY_ROLE_RESET, - itemId: role.name - }; -} - -export function modifyRole(role: Role, callback?: () => void) { - return function(dispatch: Dispatch) { - dispatch(modifyRolePending(role)); - return apiClient - .put(role._links.update.href, role, CONTENT_TYPE_ROLE) - .then(() => { - dispatch(modifyRoleSuccess(role)); - if (callback) { - callback(); +function listReducer(state: any = {}, action: any = {}) { + switch (action.type) { + case FETCH_ROLES_SUCCESS: + const roles = action.payload._embedded.repositoryRoles; + const roleNames = roles.map(role => role.name); + return { + ...state, + entries: roleNames, + entry: { + roleCreatePermission: !!action.payload._links.create, + page: action.payload.page, + pageTotal: action.payload.pageTotal, + _links: action.payload._links } - }) - .then(() => { - dispatch(fetchRoleByLink(role)); - }) - .catch(err => { - dispatch(modifyRoleFailure(role, err)); - }); - }; -} - -// delete role -export function deleteRolePending(role: Role): Action { - return { - type: DELETE_ROLE_PENDING, - payload: role, - itemId: role.name - }; -} - -export function deleteRoleSuccess(role: Role): Action { - return { - type: DELETE_ROLE_SUCCESS, - payload: role, - itemId: role.name - }; -} - -export function deleteRoleFailure(role: Role, error: Error): Action { - return { - type: DELETE_ROLE_FAILURE, - payload: { - error, - role - }, - itemId: role.name - }; -} - -export function deleteRole(role: Role, callback?: () => void) { - return function(dispatch: any) { - dispatch(deleteRolePending(role)); - return apiClient - .delete(role._links.delete.href) - .then(() => { - dispatch(deleteRoleSuccess(role)); - if (callback) { - callback(); - } - }) - .catch(error => { - dispatch(deleteRoleFailure(role, error)); - }); - }; + }; + default: + return state; + } } function extractRolesByNames( @@ -301,22 +210,6 @@ function extractRolesByNames( return rolesByNames; } -function deleteRoleInRolesByNames(roles: {}, roleName: string) { - let newRoles = {}; - for (let rolename in roles) { - if (rolename !== roleName) newRoles[rolename] = roles[rolename]; - } - return newRoles; -} - -function deleteRoleInEntries(roles: [], roleName: string) { - let newRoles = []; - for (let role of roles) { - if (role !== roleName) newRoles.push(role); - } - return newRoles; -} - const reducerByName = (state: any, rolename: string, newRoleState: any) => { return { ...state, @@ -324,37 +217,6 @@ const reducerByName = (state: any, rolename: string, newRoleState: any) => { }; }; -function listReducer(state: any = {}, action: any = {}) { - switch (action.type) { - case FETCH_ROLES_SUCCESS: - const roles = action.payload._embedded.repositoryRoles; - const roleNames = roles.map(role => role.name); - return { - ...state, - entries: roleNames, - entry: { - roleCreatePermission: !!action.payload._links.create, - page: action.payload.page, - pageTotal: action.payload.pageTotal, - _links: action.payload._links - } - }; - - // Delete single role actions - case DELETE_ROLE_SUCCESS: - const newRoleEntries = deleteRoleInEntries( - state.entries, - action.payload.name - ); - return { - ...state, - entries: newRoleEntries - }; - default: - return state; - } -} - function byNamesReducer(state: any = {}, action: any = {}) { switch (action.type) { // Fetch all roles actions @@ -365,17 +227,9 @@ function byNamesReducer(state: any = {}, action: any = {}) { return { ...byNames }; - // Fetch single role actions case FETCH_ROLE_SUCCESS: return reducerByName(state, action.payload.name, action.payload); - - case DELETE_ROLE_SUCCESS: - return deleteRoleInRolesByNames( - state, - action.payload.name - ); - default: return state; } @@ -453,19 +307,3 @@ export function isFetchRolePending(state: Object, name: string) { export function getFetchRoleFailure(state: Object, name: string) { return getFailure(state, FETCH_ROLE, name); } - -export function isModifyRolePending(state: Object, name: string) { - return isPending(state, MODIFY_ROLE, name); -} - -export function getModifyRoleFailure(state: Object, name: string) { - return getFailure(state, MODIFY_ROLE, name); -} - -export function isDeleteRolePending(state: Object, name: string) { - return isPending(state, DELETE_ROLE, name); -} - -export function getDeleteRoleFailure(state: Object, name: string) { - return getFailure(state, DELETE_ROLE, name); -} diff --git a/scm-ui/src/config/modules/roles.test.js b/scm-ui/src/config/modules/roles.test.js index f7654ef38b..38da47c9ec 100644 --- a/scm-ui/src/config/modules/roles.test.js +++ b/scm-ui/src/config/modules/roles.test.js @@ -16,14 +16,6 @@ import reducer, { CREATE_ROLE_PENDING, CREATE_ROLE_SUCCESS, CREATE_ROLE_FAILURE, - MODIFY_ROLE, - MODIFY_ROLE_PENDING, - MODIFY_ROLE_SUCCESS, - MODIFY_ROLE_FAILURE, - DELETE_ROLE, - DELETE_ROLE_PENDING, - DELETE_ROLE_SUCCESS, - DELETE_ROLE_FAILURE, fetchRoles, getFetchRolesFailure, getRolesFromState, @@ -38,13 +30,6 @@ import reducer, { isCreateRolePending, getCreateRoleFailure, getRoleByName, - modifyRole, - isModifyRolePending, - getModifyRoleFailure, - deleteRole, - isDeleteRolePending, - deleteRoleSuccess, - getDeleteRoleFailure, selectListAsCollection, isPermittedToCreateRoles } from "./roles"; @@ -120,7 +105,7 @@ const ROLE1_URL = "http://localhost:8081/api/v2/repositoryRoles/specialrole"; const error = new Error("FEHLER!"); -describe("repository roles fetch()", () => { +describe("repository roles fetch", () => { const mockStore = configureMockStore([thunk]); afterEach(() => { fetchMock.reset(); @@ -262,107 +247,14 @@ describe("repository roles fetch()", () => { expect(callMe).toBe("yeah"); }); }); - - it("successfully update role", () => { - fetchMock.putOnce(ROLE1_URL, { - status: 204 - }); - fetchMock.getOnce(ROLE1_URL, role1); - - const store = mockStore({}); - return store.dispatch(modifyRole(role1)).then(() => { - const actions = store.getActions(); - expect(actions.length).toBe(3); - expect(actions[0].type).toEqual(MODIFY_ROLE_PENDING); - expect(actions[1].type).toEqual(MODIFY_ROLE_SUCCESS); - expect(actions[2].type).toEqual(FETCH_ROLE_PENDING); - }); - }); - - it("should call callback, after successful modified role", () => { - fetchMock.putOnce(ROLE1_URL, { - status: 204 - }); - fetchMock.getOnce(ROLE1_URL, role1); - - let called = false; - const callMe = () => { - called = true; - }; - - const store = mockStore({}); - return store.dispatch(modifyRole(role1, callMe)).then(() => { - expect(called).toBeTruthy(); - }); - }); - - it("should fail updating role on HTTP 500", () => { - fetchMock.putOnce(ROLE1_URL, { - status: 500 - }); - - const store = mockStore({}); - return store.dispatch(modifyRole(role1)).then(() => { - const actions = store.getActions(); - expect(actions[0].type).toEqual(MODIFY_ROLE_PENDING); - expect(actions[1].type).toEqual(MODIFY_ROLE_FAILURE); - expect(actions[1].payload).toBeDefined(); - }); - }); - - it("should delete successfully role1", () => { - fetchMock.deleteOnce(ROLE1_URL, { - status: 204 - }); - - const store = mockStore({}); - return store.dispatch(deleteRole(role1)).then(() => { - const actions = store.getActions(); - expect(actions.length).toBe(2); - expect(actions[0].type).toEqual(DELETE_ROLE_PENDING); - expect(actions[0].payload).toBe(role1); - expect(actions[1].type).toEqual(DELETE_ROLE_SUCCESS); - }); - }); - - it("should call the callback after successful delete", () => { - fetchMock.deleteOnce(ROLE1_URL, { - status: 204 - }); - - let called = false; - const callMe = () => { - called = true; - }; - - const store = mockStore({}); - return store.dispatch(deleteRole(role1, callMe)).then(() => { - expect(called).toBeTruthy(); - }); - }); - - it("should fail to delete role1", () => { - fetchMock.deleteOnce(ROLE1_URL, { - status: 500 - }); - - const store = mockStore({}); - return store.dispatch(deleteRole(role1)).then(() => { - const actions = store.getActions(); - expect(actions[0].type).toEqual(DELETE_ROLE_PENDING); - expect(actions[0].payload).toBe(role1); - expect(actions[1].type).toEqual(DELETE_ROLE_FAILURE); - expect(actions[1].payload).toBeDefined(); - }); - }); }); -describe("roles reducer", () => { +describe("repository roles reducer", () => { it("should update state correctly according to FETCH_ROLES_SUCCESS action", () => { const newState = reducer({}, fetchRolesSuccess(responseBody)); expect(newState.list).toEqual({ - entries: ["SPECIALROLE", "WRITE"], + entries: ["specialrole", "WRITE"], entry: { roleCreatePermission: true, page: 0, @@ -372,7 +264,7 @@ describe("roles reducer", () => { }); expect(newState.byNames).toEqual({ - SPECIALROLE: role1, + specialrole: role1, WRITE: role2 }); @@ -397,23 +289,6 @@ describe("roles reducer", () => { expect(newState.byNames["WRITE"]).toBeDefined(); }); - it("should remove role from state when delete succeeds", () => { - const state = { - list: { - entries: ["WRITE", "specialrole"] - }, - byNames: { - specialrole: role1, - WRITE: role2 - } - }; - - const newState = reducer(state, deleteRoleSuccess(role2)); - expect(newState.byNames["specialrole"]).toBeDefined(); - expect(newState.byNames["WRITE"]).toBeFalsy(); - expect(newState.list.entries).toEqual(["specialrole"]); - }); - it("should set roleCreatePermission to true if create link is present", () => { const newState = reducer({}, fetchRolesSuccess(responseBody)); @@ -442,7 +317,7 @@ describe("roles reducer", () => { }); }); -describe("selector tests", () => { +describe("repository roles selector", () => { it("should return an empty object", () => { expect(selectListAsCollection({})).toEqual({}); expect(selectListAsCollection({ repositoryRoles: { a: "a" } })).toEqual({}); @@ -597,56 +472,4 @@ describe("selector tests", () => { it("should return undefined when fetch role2 did not fail", () => { expect(getFetchRoleFailure({}, "role2")).toBe(undefined); }); - - it("should return true, when modify role1 is pending", () => { - const state = { - pending: { - [MODIFY_ROLE + "/role1"]: true - } - }; - expect(isModifyRolePending(state, "role1")).toEqual(true); - }); - - it("should return false, when modify role1 is not pending", () => { - expect(isModifyRolePending({}, "role1")).toEqual(false); - }); - - it("should return error when modify role1 did fail", () => { - const state = { - failure: { - [MODIFY_ROLE + "/role1"]: error - } - }; - expect(getModifyRoleFailure(state, "role1")).toEqual(error); - }); - - it("should return undefined when modify role1 did not fail", () => { - expect(getModifyRoleFailure({}, "role1")).toBe(undefined); - }); - - it("should return true, when delete role2 is pending", () => { - const state = { - pending: { - [DELETE_ROLE + "/role2"]: true - } - }; - expect(isDeleteRolePending(state, "role2")).toEqual(true); - }); - - it("should return false, when delete role2 is not pending", () => { - expect(isDeleteRolePending({}, "role2")).toEqual(false); - }); - - it("should return error when delete role2 did fail", () => { - const state = { - failure: { - [DELETE_ROLE + "/role2"]: error - } - }; - expect(getDeleteRoleFailure(state, "role2")).toEqual(error); - }); - - it("should return undefined when delete role2 did not fail", () => { - expect(getDeleteRoleFailure({}, "role2")).toBe(undefined); - }); });